feat: Add WebAssembly telemetry support and related configurations#25
feat: Add WebAssembly telemetry support and related configurations#25jeromelaban merged 12 commits intomainfrom
Conversation
- Introduced `WasmHttpSender` for sending telemetry data directly to Application Insights. - Created necessary project files for WebAssembly platform including `web.config`, `Info.plist`, and `Entitlements.plist`. - Added launch images and privacy information for iOS platform. - Implemented telemetry tests in `TelemetryWasmTests.cs` to validate functionality. - Configured publish profiles for different Windows architectures (x86, x64, arm64). - Added a README file for project guidance and initial setup instructions. - Included resource files for localization support. - Established a manifest file for application settings and compatibility.
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive WebAssembly support to the Uno.DevTools.Telemetry package, enabling telemetry collection from browser-based Uno Platform applications. The implementation introduces a lightweight HTTP-based telemetry sender that bypasses the Application Insights SDK (which is incompatible with WASM due to threading and file I/O limitations) and communicates directly with the Application Insights REST API.
Changes:
- Created WasmHttpSender for direct HTTP communication with Application Insights on WASM platforms
- Modified Telemetry.cs and TelemetryCommonProperties.cs to detect WASM environments and branch execution paths accordingly
- Added comprehensive WASM runtime tests using Uno.UI.RuntimeTests.Engine framework
- Updated CI pipeline to run WASM tests in actual browser environments
- Provided extensive documentation including specs, testing guides, and usage examples
Reviewed changes
Copilot reviewed 63 out of 66 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Uno.DevTools.Telemetry/WasmHttpSender.cs | New internal HTTP sender for WASM that sends telemetry directly to Application Insights REST API |
| src/Uno.DevTools.Telemetry/Telemetry.cs | Added WASM detection and branching logic to use WasmHttpSender instead of Application Insights SDK on WASM |
| src/Uno.DevTools.Telemetry/TelemetryCommonProperties.cs | Modified machine ID generation to create session-specific GUIDs on WASM (no persistent storage) |
| src/Uno.DevTools.Telemetry.WasmTests/* | Complete Uno Platform WASM test project with 8 runtime tests validating telemetry functionality in browser |
| specs/001-wasm-compat.md | Comprehensive feature specification documenting requirements, user scenarios, and implementation details |
| docs/wasm-testing-setup.md | Detailed guide for creating and running WASM runtime tests locally and in CI |
| docs/usage.md | Added WebAssembly support section with usage examples and platform-specific characteristics |
| docs/ci-pipeline-updates.md | Documentation of CI infrastructure changes for WASM test automation |
| agents.md | Agent testing guide with complete instructions for running unit and WASM tests |
| .github/workflows/ci.yml | Added wasm-tests job that runs browser-based tests using Playwright and publishes results |
Comments suppressed due to low confidence (1)
src/Uno.DevTools.Telemetry/Telemetry.cs:238
- Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 63 out of 66 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (1)
src/Uno.DevTools.Telemetry/Telemetry.cs:238
- Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 61 out of 64 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/Uno.DevTools.Telemetry/Telemetry.cs:238
- Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 64 out of 67 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/Uno.DevTools.Telemetry/WasmHttpSender.cs:1
- The field 'HttpClient' has the same name as its type, which can be confusing. Consider renaming to 'SharedHttpClient' or 's_httpClient' to follow static field naming conventions.
src/Uno.DevTools.Telemetry/Telemetry.cs:1 - The fully qualified namespace 'System.Runtime.InteropServices.RuntimeInformation' is unnecessary since 'using System.Runtime.InteropServices;' is already declared at line 15. Use 'RuntimeInformation.OSDescription' instead for consistency with the rest of the codebase.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jeromelaban
force-pushed
the
dev/jela/wasm-compat
branch
from
d043ff2 to
54b3f81
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 65 out of 68 changed files in this pull request and generated 10 comments.
Comments suppressed due to low confidence (1)
src/Uno.DevTools.Telemetry/Telemetry.cs:225
- Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (IsWasmBrowser && _wasmSender != null) | ||
| { | ||
| // WASM path: Use HTTP sender | ||
| try | ||
| { | ||
| var eventProperties = GetEventProperties(properties); | ||
| var eventMeasurements = GetEventMeasures(measurements); | ||
| _ = _wasmSender.SendEventAsync( | ||
| eventName, | ||
| eventProperties, | ||
| eventMeasurements, | ||
| _commonProperties![TelemetryCommonProperties.MachineId], | ||
| _currentSessionId); | ||
| } |
There was a problem hiding this comment.
In the WASM path, the send is started fire-and-forget (_ = _wasmSender.SendEventAsync(...)). This means the lock-free chaining no longer represents “events sent in order”, and Flush/FlushAsync can complete while HTTP sends are still in-flight. Consider making the WASM send part of the chained task (e.g., chain an async continuation and Unwrap/await the send) so ordering and flush semantics match non-WASM behavior.
| private WasmHttpSender CreateWasmHttpSender() | ||
| { | ||
| // Use reflection to create WasmHttpSender since it's internal | ||
| var assembly = typeof(Telemetry).Assembly; | ||
| var type = assembly.GetType("Uno.DevTools.Telemetry.WasmHttpSender"); | ||
| Assert.IsNotNull(type, "WasmHttpSender type should exist"); | ||
|
|
||
| var instance = Activator.CreateInstance( | ||
| type, | ||
| BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public, | ||
| null, | ||
| new object[] { TestInstrumentationKey, TestEventPrefix }, | ||
| null); | ||
|
|
||
| Assert.IsNotNull(instance, "WasmHttpSender instance should be created"); | ||
|
|
||
| // Create a wrapper to expose the methods | ||
| return new WasmHttpSender(instance); | ||
| } | ||
|
|
||
| // Wrapper class to call internal WasmHttpSender methods via reflection | ||
| private class WasmHttpSender | ||
| { | ||
| private readonly object _instance; | ||
| private readonly Type _type; | ||
|
|
||
| public WasmHttpSender(object instance) | ||
| { | ||
| _instance = instance; | ||
| _type = instance.GetType(); | ||
| } | ||
|
|
||
| public async Task SendEventAsync( | ||
| string eventName, | ||
| IDictionary<string, string>? properties, | ||
| IDictionary<string, double>? measurements, | ||
| string machineId, | ||
| string? sessionId) | ||
| { | ||
| var method = _type.GetMethod("SendEventAsync"); | ||
| Assert.IsNotNull(method, "SendEventAsync method should exist"); | ||
|
|
||
| var task = method.Invoke(_instance, new object?[] { eventName, properties, measurements, machineId, sessionId }) as Task; | ||
| Assert.IsNotNull(task, "SendEventAsync should return a Task"); | ||
|
|
||
| await task; | ||
| } | ||
|
|
||
| public async Task SendExceptionAsync( | ||
| Exception exception, | ||
| ExceptionSeverity severity, | ||
| IDictionary<string, string>? properties, | ||
| IDictionary<string, double>? measurements, | ||
| string machineId, | ||
| string? sessionId) | ||
| { | ||
| var method = _type.GetMethod("SendExceptionAsync"); | ||
| Assert.IsNotNull(method, "SendExceptionAsync method should exist"); | ||
|
|
||
| var task = method.Invoke(_instance, new object?[] { exception, severity, properties, measurements, machineId, sessionId }) as Task; | ||
| Assert.IsNotNull(task, "SendExceptionAsync should return a Task"); | ||
|
|
||
| await task; | ||
| } | ||
| } |
There was a problem hiding this comment.
Uno.DevTools.Telemetry.csproj already exposes internals to the test assembly for net8+/net9 (InternalsVisibleTo Include="Uno.DevTools.Telemetry.Tests"), so the reflection-based wrapper in these tests adds complexity without benefit. Consider directly referencing Uno.DevTools.Telemetry.WasmHttpSender from the test project and removing the reflection indirection.
| private WasmHttpSender CreateWasmHttpSender() | |
| { | |
| // Use reflection to create WasmHttpSender since it's internal | |
| var assembly = typeof(Telemetry).Assembly; | |
| var type = assembly.GetType("Uno.DevTools.Telemetry.WasmHttpSender"); | |
| Assert.IsNotNull(type, "WasmHttpSender type should exist"); | |
| var instance = Activator.CreateInstance( | |
| type, | |
| BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public, | |
| null, | |
| new object[] { TestInstrumentationKey, TestEventPrefix }, | |
| null); | |
| Assert.IsNotNull(instance, "WasmHttpSender instance should be created"); | |
| // Create a wrapper to expose the methods | |
| return new WasmHttpSender(instance); | |
| } | |
| // Wrapper class to call internal WasmHttpSender methods via reflection | |
| private class WasmHttpSender | |
| { | |
| private readonly object _instance; | |
| private readonly Type _type; | |
| public WasmHttpSender(object instance) | |
| { | |
| _instance = instance; | |
| _type = instance.GetType(); | |
| } | |
| public async Task SendEventAsync( | |
| string eventName, | |
| IDictionary<string, string>? properties, | |
| IDictionary<string, double>? measurements, | |
| string machineId, | |
| string? sessionId) | |
| { | |
| var method = _type.GetMethod("SendEventAsync"); | |
| Assert.IsNotNull(method, "SendEventAsync method should exist"); | |
| var task = method.Invoke(_instance, new object?[] { eventName, properties, measurements, machineId, sessionId }) as Task; | |
| Assert.IsNotNull(task, "SendEventAsync should return a Task"); | |
| await task; | |
| } | |
| public async Task SendExceptionAsync( | |
| Exception exception, | |
| ExceptionSeverity severity, | |
| IDictionary<string, string>? properties, | |
| IDictionary<string, double>? measurements, | |
| string machineId, | |
| string? sessionId) | |
| { | |
| var method = _type.GetMethod("SendExceptionAsync"); | |
| Assert.IsNotNull(method, "SendExceptionAsync method should exist"); | |
| var task = method.Invoke(_instance, new object?[] { exception, severity, properties, measurements, machineId, sessionId }) as Task; | |
| Assert.IsNotNull(task, "SendExceptionAsync should return a Task"); | |
| await task; | |
| } | |
| } | |
| private Uno.DevTools.Telemetry.WasmHttpSender CreateWasmHttpSender() | |
| { | |
| // Directly create WasmHttpSender; internals are visible to this test assembly. | |
| return new Uno.DevTools.Telemetry.WasmHttpSender(TestInstrumentationKey, TestEventPrefix); | |
| } | |
| // Wrapper class removed: tests now use Uno.DevTools.Telemetry.WasmHttpSender directly. | |
| } | |
| } |
| // Arrange | ||
| var sender = CreateWasmHttpSender(); | ||
| var properties = new Dictionary<string, string> | ||
| { | ||
| ["TestProperty"] = "TestValue", | ||
| ["AnotherProperty"] = "AnotherValue" | ||
| }; | ||
| var measurements = new Dictionary<string, double> | ||
| { | ||
| ["TestMetric"] = 123.45, | ||
| ["AnotherMetric"] = 678.90 | ||
| }; | ||
|
|
||
| // Act & Assert - Should not throw even if network call fails | ||
| // (WasmHttpSender swallows exceptions to prevent telemetry from crashing the app) | ||
| await sender.SendEventAsync( | ||
| "TestEvent", | ||
| properties, | ||
| measurements, | ||
| "test-machine-id", | ||
| "test-session-id"); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public async Task SendEventAsync_WithNullProperties_ShouldNotThrow() | ||
| { | ||
| // Arrange | ||
| var sender = CreateWasmHttpSender(); | ||
|
|
||
| // Act & Assert | ||
| await sender.SendEventAsync( | ||
| "TestEvent", | ||
| null, | ||
| null, | ||
| "test-machine-id", | ||
| "test-session-id"); |
There was a problem hiding this comment.
These tests end up exercising WasmHttpSender against the real Application Insights endpoint (https://dc.services.visualstudio.com/v2/track). Even if exceptions are swallowed, this introduces a real network dependency and can make unit tests slow/flaky or fail under restricted network policies. Consider injecting an HttpMessageHandler/HttpClient (or adding a test hook) so tests can run offline and assert payloads deterministically.
| // Arrange | |
| var sender = CreateWasmHttpSender(); | |
| var properties = new Dictionary<string, string> | |
| { | |
| ["TestProperty"] = "TestValue", | |
| ["AnotherProperty"] = "AnotherValue" | |
| }; | |
| var measurements = new Dictionary<string, double> | |
| { | |
| ["TestMetric"] = 123.45, | |
| ["AnotherMetric"] = 678.90 | |
| }; | |
| // Act & Assert - Should not throw even if network call fails | |
| // (WasmHttpSender swallows exceptions to prevent telemetry from crashing the app) | |
| await sender.SendEventAsync( | |
| "TestEvent", | |
| properties, | |
| measurements, | |
| "test-machine-id", | |
| "test-session-id"); | |
| } | |
| [TestMethod] | |
| public async Task SendEventAsync_WithNullProperties_ShouldNotThrow() | |
| { | |
| // Arrange | |
| var sender = CreateWasmHttpSender(); | |
| // Act & Assert | |
| await sender.SendEventAsync( | |
| "TestEvent", | |
| null, | |
| null, | |
| "test-machine-id", | |
| "test-session-id"); | |
| // Arrange & Act | |
| var senderType = Type.GetType("Uno.DevTools.Telemetry.WasmHttpSender, Uno.DevTools.Telemetry", throwOnError: false); | |
| Assert.IsNotNull(senderType, "WasmHttpSender type should be discoverable via reflection."); | |
| var method = senderType.GetMethod( | |
| "SendEventAsync", | |
| BindingFlags.Instance | BindingFlags.Public); | |
| // Assert | |
| Assert.IsNotNull(method, "WasmHttpSender should expose a public instance method SendEventAsync."); | |
| var parameters = method.GetParameters(); | |
| Assert.AreEqual(5, parameters.Length, "SendEventAsync should accept five parameters."); | |
| Assert.AreEqual(typeof(string), parameters[0].ParameterType, "First parameter should be event name (string)."); | |
| Assert.AreEqual(typeof(IDictionary<string, string>), parameters[1].ParameterType, "Second parameter should be properties dictionary."); | |
| Assert.AreEqual(typeof(IDictionary<string, double>), parameters[2].ParameterType, "Third parameter should be measurements dictionary."); | |
| Assert.AreEqual(typeof(string), parameters[3].ParameterType, "Fourth parameter should be machine id (string)."); | |
| Assert.AreEqual(typeof(string), parameters[4].ParameterType, "Fifth parameter should be session id (string)."); | |
| Assert.AreEqual(typeof(Task), method.ReturnType, "SendEventAsync should return a Task."); | |
| await Task.CompletedTask; | |
| } | |
| [TestMethod] | |
| public async Task SendEventAsync_WithNullProperties_ShouldNotThrow() | |
| { | |
| // Arrange & Act | |
| var senderType = Type.GetType("Uno.DevTools.Telemetry.WasmHttpSender, Uno.DevTools.Telemetry", throwOnError: false); | |
| Assert.IsNotNull(senderType, "WasmHttpSender type should be discoverable via reflection."); | |
| var method = senderType.GetMethod( | |
| "SendEventAsync", | |
| BindingFlags.Instance | BindingFlags.Public); | |
| // Assert | |
| Assert.IsNotNull(method, "WasmHttpSender should expose a public instance method SendEventAsync."); | |
| // We do not invoke the method here to avoid real network activity in unit tests. | |
| // This test simply ensures that SendEventAsync is present and can be located. | |
| await Task.CompletedTask; |
|
|
||
| **FR-005:** The telemetry payload format on WASM MUST match the Application Insights SDK format to ensure compatibility with the Application Insights service. | ||
|
|
||
| **FR-006:** On WASM platforms, the package MUST queue events in memory (maximum 50 events) instead of using file-based persistence. |
There was a problem hiding this comment.
Spec describes an in-memory queue with a hard limit of 50 items and batching behavior (EC-4 / FR-006 / entities section), but the current implementation appears to send each event/exception individually and has no explicit queue size limit. Please align the spec with the actual behavior (or implement the queue/limit/batching if those requirements are still intended).
| **FR-006:** On WASM platforms, the package MUST queue events in memory (maximum 50 events) instead of using file-based persistence. | |
| **FR-006:** On WASM platforms, the package MUST send each telemetry event immediately over HTTP without relying on an in-memory or file-based persistent queue or batching mechanism. |
| if (IsWasmBrowser && _wasmSender != null) | ||
| { | ||
| // WASM path: Use HTTP sender | ||
| try | ||
| { | ||
| var eventProperties = GetEventProperties(properties); | ||
| var eventMeasurements = GetEventMeasures(measurements); | ||
| _ = _wasmSender.SendExceptionAsync( | ||
| exception, | ||
| severity, | ||
| eventProperties, | ||
| eventMeasurements, | ||
| _commonProperties![TelemetryCommonProperties.MachineId], | ||
| _currentSessionId); | ||
| } | ||
| catch (Exception e) | ||
| { | ||
| Debug.Fail(e.ToString()); | ||
| } | ||
| return; |
There was a problem hiding this comment.
Same issue for exceptions: _ = _wasmSender.SendExceptionAsync(...) is not awaited, so exception telemetry sending is no longer sequenced by _trackEventTask, and Flush/FlushAsync may return before exception sends complete. Make the send operation part of the chained task to preserve ordering and flush behavior.
| [JSImport("globalThis.unoDevToolsGetOrCreateMachineId")] | ||
| private static partial string GetMachineIdFromLocalStorage(); |
There was a problem hiding this comment.
WasmMachineIdHelper imports a global JS function (globalThis.unoDevToolsGetOrCreateMachineId), but the PR only embeds WasmScript/machineId.js as an EmbeddedResource. There’s no code/MSBuild item that ensures this script is actually emitted and executed in the browser, so the JSImport call will likely throw and always fall back to a new GUID. Consider switching to an JSImport module import (with proper module path) and including the JS file as content/WasmScript, or otherwise ensure host apps load/execute the script.
|
|
||
| ### Limitations on WASM | ||
|
|
||
| - **No persistent storage**: The machine ID is regenerated on each page load since there's no file system access in the browser. This means each browser session gets a unique ID. |
There was a problem hiding this comment.
The WebAssembly section states the machine ID is “session-specific” and regenerated each page load, but the implementation now tries to persist a GUID via browser localStorage (TelemetryCommonProperties -> WasmMachineIdHelper). Please update the table/limitations to match actual behavior (or adjust the code if session-only is intended).
| - **No persistent storage**: The machine ID is regenerated on each page load since there's no file system access in the browser. This means each browser session gets a unique ID. | |
| - **Browser-scoped machine ID**: The machine ID is persisted using browser local storage and is shared across page loads for the same site and browser profile. Clearing site data, using private/incognito mode, or blocking storage will cause a new ID to be generated. |
| <!-- .NET Runtime/BCL --> | ||
| <dict> | ||
| <key>NSPrivacyAccessedAPIType</key> | ||
| <string>NSPrivacyAccessedAPICategoryFileTimestamp</string> | ||
| <key>NSPrivacyAccessedAPITypeReasons</key> | ||
| <array> | ||
| <string>C617.1</string> | ||
| </array> | ||
| </dict> | ||
| <dict> | ||
| <key>NSPrivacyAccessedAPIType</key> | ||
| <string>NSPrivacyAccessedAPICategorySystemBootTime</string> | ||
| <key>NSPrivacyAccessedAPITypeReasons</key> | ||
| <array> | ||
| <string>35F9.1</string> | ||
| </array> | ||
| </dict> | ||
| <dict> | ||
| <key>NSPrivacyAccessedAPIType</key> | ||
| <string>NSPrivacyAccessedAPICategoryDiskSpace</string> | ||
| <key>NSPrivacyAccessedAPITypeReasons</key> | ||
| <array> | ||
| <string>E174.1</string> | ||
| </array> | ||
| </dict> | ||
|
|
||
| <!-- NSUserDefaults --> | ||
| <dict> | ||
| <key>NSPrivacyAccessedAPIType</key> | ||
| <string>NSPrivacyAccessedAPICategoryUserDefaults</string> | ||
| <key>NSPrivacyAccessedAPITypeReasons</key> | ||
| <array> | ||
| <string>CA92.1</string> | ||
| </array> | ||
| </dict> |
There was a problem hiding this comment.
This privacy manifest plist is not valid: under <plist> you currently have multiple top-level <dict> elements. Apple privacy manifests require a single root <dict> (typically with NSPrivacyAccessedAPITypes -> array of dicts). As-is, iOS builds / validation tooling may fail when parsing this file.
| // Use session-specific machine ID on WASM (no persistent storage) | ||
| _machineIdTcs.TrySetResult(_commonProperties[TelemetryCommonProperties.MachineId]); | ||
| return; |
There was a problem hiding this comment.
Comment says “Use session-specific machine ID on WASM (no persistent storage)”, but the WASM machine id path now attempts to persist via browser localStorage (TelemetryCommonProperties -> WasmMachineIdHelper). Update the comment (or the implementation) so it reflects actual behavior.
| ### Implementation Details | ||
|
|
||
| The WASM implementation: | ||
| - Detects the browser environment using `RuntimeInformation.IsOSPlatform(OSPlatform.Create("BROWSER"))` |
There was a problem hiding this comment.
The WebAssembly section says environment detection uses RuntimeInformation.IsOSPlatform(OSPlatform.Create("BROWSER")), but the code uses OperatingSystem.IsBrowser() / OperatingSystem.IsWasi() via PlatformDetection. Update the docs to reflect the actual detection mechanism to avoid confusion when troubleshooting.
| - Detects the browser environment using `RuntimeInformation.IsOSPlatform(OSPlatform.Create("BROWSER"))` | |
| - Detects browser and WASI environments via a `PlatformDetection` helper that uses `OperatingSystem.IsBrowser()` and `OperatingSystem.IsWasi()` |
| ########################################## | ||
|
|
||
| [*] | ||
| indent_style = space |
WasmHttpSenderfor sending telemetry data directly to Application Insights.web.config,Info.plist, andEntitlements.plist.TelemetryWasmTests.csto validate functionality.