From 05b535d09cd0404e00acb98244d07581cad1c456 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Mon, 7 Jul 2025 11:44:58 -0400 Subject: [PATCH 1/8] chore: Improve solution structure to add global files visible in IDE --- src/Uno.DevTools.Telemetry.sln | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/Uno.DevTools.Telemetry.sln b/src/Uno.DevTools.Telemetry.sln index daa5bf6..b19a75d 100644 --- a/src/Uno.DevTools.Telemetry.sln +++ b/src/Uno.DevTools.Telemetry.sln @@ -5,6 +5,16 @@ VisualStudioVersion = 17.13.35507.96 d17.13 MinimumVisualStudioVersion = 10.0.40219.1 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Uno.DevTools.Telemetry", "Uno.DevTools.Telemetry\Uno.DevTools.Telemetry.csproj", "{291C2E10-5CAA-465B-A14E-CB7954AB25A8}" EndProject +Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Files", "Files", "{A7C5A7B2-2558-4941-AA1E-3B332819197B}" + ProjectSection(SolutionItems) = preProject + Directory.Build.props = Directory.Build.props + Directory.Build.targets = Directory.Build.targets + Directory.Packages.props = Directory.Packages.props + global.json = global.json + ..\README.md = ..\README.md + ..\.github\workflows\ci.yml = ..\.github\workflows\ci.yml + EndProjectSection +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU From db9f257e4e91383d6a76792402a3ea8b8f0a012e Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Mon, 7 Jul 2025 12:13:26 -0400 Subject: [PATCH 2/8] docs: Adjust README for the project --- README.md | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 58 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 0dfbac6..a706241 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,61 @@ -# Uno Platform Developer tools for Telemetry +# Uno.DevTools.Telemetry -Learn how to [get started with Uno Platform](https://aka.platform.uno/get-started). +Uno.DevTools.Telemetry is a .NET library for collecting and sending telemetry data, designed for integration with Uno Platform development tools and other .NET applications. -This repository contains the telemetry source used through Uno Platform's developer tools, derived from the existing `dotnet.exe` telemetry. +## Features +- Supports Microsoft Application Insights +- Extensible and testable architecture +- File-based telemetry channel for local development +- High-performance, immutable code style +- Modern C# features and best practices + +## Installation +Add the NuGet package to your project: + +```shell +dotnet add package Uno.DevTools.Telemetry +``` + +## Usage + +### Without DI + +```csharp +const string instrumentationKey = ""; // GUID from AppInsight +const string analyticsEventPrefix = "uno/my-project"; // prefix for every analytics event + +var telemetry = new Telemetry( + instrumentationKey, + analyticsEventPrefix, + typeof(MyProject).Assembly +); +``` + +### Tracking Events + +```csharp +var telemetry = serviceProvider.GetRequiredService(); +telemetry.TrackEvent("AppStarted"); +telemetry.TrackEvent("UserAction", new Dictionary { { "Action", "Clicked" } }, null); +``` + +### File-based Telemetry +By default, telemetry is persisted locally before being sent. You can configure the storage location and behavior by customizing the `Telemetry` constructor. + +## Environment Variables +- `UNO_PLATFORM_TELEMETRY_OPTOUT`: Set to `true` to disable telemetry. + +## Development +- All code and comments are in English. +- Follows strict code style and performance guidelines (see `agent-instructions.md`). +- Tasks and project status are tracked in `todos.md` (local only). +- Do not disable analyzers. +- See also: `CODE_OF_CONDUCT.md`, `SECURITY.md`, `LICENSE.md`. + +## License +Apache 2.0 License. See `LICENSE.md` for details. + +--- + +*For more details, see the code and comments in the repository.* -The package generated from this repository is not integrated into end-user apps, and should be not be referenced directly. From 00e3c1a2f02d2199dcb67fea1f65bcdf8f31a9bb Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Mon, 7 Jul 2025 12:16:08 -0400 Subject: [PATCH 3/8] feat(telemetry): add FileTelemetry and DI extension - Added FileTelemetry implementation - Conditional DI extension for file-based telemetry - Updated documentation and environment variable support - NuGet packaging/license and dependency fixes --- README.md | 69 ++++++++- src/Directory.Packages.props | 16 +- src/Uno.DevTools.Telemetry/FileTelemetry.cs | 142 ++++++++++++++++++ src/Uno.DevTools.Telemetry/HashBuilder.cs | 37 ++--- .../TelemetryServiceCollectionExtensions.cs | 52 +++++++ .../Uno.DevTools.Telemetry.csproj | 8 +- 6 files changed, 285 insertions(+), 39 deletions(-) create mode 100644 src/Uno.DevTools.Telemetry/FileTelemetry.cs create mode 100644 src/Uno.DevTools.Telemetry/TelemetryServiceCollectionExtensions.cs diff --git a/README.md b/README.md index a706241..0f16e02 100644 --- a/README.md +++ b/README.md @@ -3,6 +3,7 @@ Uno.DevTools.Telemetry is a .NET library for collecting and sending telemetry data, designed for integration with Uno Platform development tools and other .NET applications. ## Features +- Optional but easy integration with .NET dependency injection (`IServiceCollection`) - Supports Microsoft Application Insights - Extensible and testable architecture - File-based telemetry channel for local development @@ -31,6 +32,26 @@ var telemetry = new Telemetry( ); ``` +### Registering Telemetry in DI + +```csharp +using Uno.DevTools.Telemetry; + +var services = new ServiceCollection(); +services.AddTelemetry( + instrumentationKey: "", + eventNamePrefix: "MyApp/Telemetry", + versionAssembly: typeof(MyProject).Assembly, + sessionId: "optional-session-id" +); +``` + +#### Parameters for `AddTelemetry` +- `instrumentationKey` (**required**): Application Insights instrumentation key (GUID). +- `eventNamePrefix` (optional): Prefix for all telemetry events (e.g. `uno/my-project`). +- `versionAssembly` (optional): Assembly used for version info (defaults to calling assembly). +- `sessionId` (optional): Custom session id. + ### Tracking Events ```csharp @@ -44,18 +65,50 @@ By default, telemetry is persisted locally before being sent. You can configure ## Environment Variables - `UNO_PLATFORM_TELEMETRY_OPTOUT`: Set to `true` to disable telemetry. +- `UNO_PLATFORM_TELEMETRY_FILE`: If set, telemetry will be logged to the specified file path using `FileTelemetry` instead of Application Insights. On .NET 8+, FileTelemetry uses `TimeProvider` for testable timestamps; on .NET Standard 2.0, it falls back to system time. +- `UNO_PLATFORM_TELEMETRY_SESSIONID`: (optional) Override the session id for telemetry events. -## Development -- All code and comments are in English. -- Follows strict code style and performance guidelines (see `agent-instructions.md`). -- Tasks and project status are tracked in `todos.md` (local only). -- Do not disable analyzers. -- See also: `CODE_OF_CONDUCT.md`, `SECURITY.md`, `LICENSE.md`. +> **Note:** +> - The use of `UNO_PLATFORM_TELEMETRY_FILE` is intended for testing, debugging, or local development scenarios. To activate file-based telemetry, you must use the DI extension (`AddTelemetry`) so that the environment variable is detected and the correct implementation is injected. +> - FileTelemetry is thread-safe and writes events as single-line JSON, prefixed by context if provided. +> - Multi-framework support: On .NET 8+ and .NET 9+, `FileTelemetry` uses `TimeProvider` for testable timestamps. On netstandard2.0, it falls back to `DateTime.Now`. + +## FileTelemetry & Testing + +When `UNO_PLATFORM_TELEMETRY_FILE` is set, all telemetry events are written to the specified file. This is especially useful for automated tests, CI validation, or debugging telemetry output locally. The file will contain one JSON object per line, optionally prefixed by the context (e.g., session or connection id). + +Example output: +``` +global: {"Timestamp":"2025-07-07T12:34:56.789Z","EventName":"AppStarted","Properties":{},"Measurements":null} +``` -## License -Apache 2.0 License. See `LICENSE.md` for details. +## Crash/Exception Reporting + +Crash and exception reporting is planned for a future release. For now, only explicit event tracking is supported. See `todos.md` for roadmap. + +## Multi-framework Support + +Uno.DevTools.Telemetry targets: +- .NET Standard 2.0 (broad compatibility) +- .NET 8.0 +- .NET 9.0 + +All features are available on .NET 8+; some features (like testable time via `TimeProvider`) are not available on netstandard2.0 and will fallback to system time. --- *For more details, see the code and comments in the repository.* +### Example: Using FileTelemetry from the Command Line (PowerShell) + +To log telemetry events to a file for testing or debugging, set the environment variable before launching your application (the application must use Uno.DevTools.Telemetry via DI): + +```powershell +$env:UNO_PLATFORM_TELEMETRY_FILE = "telemetry.log" +dotnet run --project path/to/YourApp.csproj +``` + +Replace `path/to/YourApp.csproj` with the path to your application's project file. All telemetry events will be written to `telemetry.log` in the working directory. + +> **Tip:** You can also specify an absolute path for the log file (e.g., `C:\temp\telemetry.log`). + diff --git a/src/Directory.Packages.props b/src/Directory.Packages.props index cb395ca..8f0fa16 100644 --- a/src/Directory.Packages.props +++ b/src/Directory.Packages.props @@ -1,9 +1,11 @@ - - - - - - - + + + + + + + + + \ No newline at end of file diff --git a/src/Uno.DevTools.Telemetry/FileTelemetry.cs b/src/Uno.DevTools.Telemetry/FileTelemetry.cs new file mode 100644 index 0000000..90ab124 --- /dev/null +++ b/src/Uno.DevTools.Telemetry/FileTelemetry.cs @@ -0,0 +1,142 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Text.Json; +using System.Threading; +using System.Threading.Tasks; + +namespace Uno.DevTools.Telemetry +{ + /// + /// A telemetry implementation that writes events to a file for testing purposes. + /// Can use either individual files per context or a single file with prefixes. + /// + public class FileTelemetry : ITelemetry + { + private static readonly JsonSerializerOptions JsonOptions = new() + { + WriteIndented = false // Use single-line JSON for easier parsing in tests + }; + + private readonly string _filePath; + private readonly string _contextPrefix; + private readonly object _lock = new(); +#if NET8_0_OR_GREATER + private readonly TimeProvider _timeProvider; +#endif + + /// + /// Creates a FileTelemetry instance with a contextual file name or prefix. + /// + /// The base file path (with or without extension) + /// The telemetry context (e.g., "global") - used when multiple instances are required, to differentiate them in the output +#if NET8_0_OR_GREATER + /// Optional time provider for testability (defaults to TimeProvider.System) + public FileTelemetry(string baseFilePath, string context, TimeProvider? timeProvider = null) +#else + public FileTelemetry(string baseFilePath, string context) +#endif + { + if (string.IsNullOrEmpty(baseFilePath)) + throw new ArgumentNullException(nameof(baseFilePath)); + if (string.IsNullOrEmpty(context)) + throw new ArgumentNullException(nameof(context)); + + _filePath = baseFilePath; + _contextPrefix = context; +#if NET8_0_OR_GREATER + _timeProvider = timeProvider ?? TimeProvider.System; +#endif + EnsureDirectoryExists(_filePath); + } + + private static void EnsureDirectoryExists(string filePath) + { + var directory = Path.GetDirectoryName(filePath); + if (!string.IsNullOrEmpty(directory) && !Directory.Exists(directory)) + { + Directory.CreateDirectory(directory); + } + } + + public bool Enabled => true; + + public void Dispose() + { + // Don't dispose to allow post-shutdown logging + } + + public void Flush() + { + // File-based telemetry doesn't need explicit flushing as we write immediately + } + + public Task FlushAsync(CancellationToken ct) => Task.CompletedTask; + + public void ThreadBlockingTrackEvent(string eventName, IDictionary properties, IDictionary measurements) + { + TrackEvent(eventName, properties, measurements); + } + + public void TrackEvent(string eventName, (string key, string value)[]? properties, (string key, double value)[]? measurements) + { + var propertiesDict = properties != null ? new Dictionary() : null; + if (properties != null) + { + foreach (var (key, value) in properties) + { + propertiesDict![key] = value; + } + } + + var measurementsDict = measurements != null ? new Dictionary() : null; + if (measurements != null) + { + foreach (var (key, value) in measurements) + { + measurementsDict![key] = value; + } + } + + TrackEvent(eventName, propertiesDict, measurementsDict); + } + + public void TrackEvent(string eventName, IDictionary? properties, IDictionary? measurements) + { + // Add the context prefix to properties if specified + var finalProperties = properties; + if (!string.IsNullOrEmpty(_contextPrefix)) + { + // Clone properties and add context + finalProperties = properties != null ? new Dictionary(properties) : new Dictionary(); + } + + var telemetryEvent = new + { +#if NET8_0_OR_GREATER + Timestamp = _timeProvider.GetLocalNow().DateTime, // Use TimeProvider for testability +#else + Timestamp = DateTime.Now, // Fallback for netstandard2.0 +#endif + EventName = eventName, + Properties = finalProperties, + Measurements = measurements + }; + + var json = JsonSerializer.Serialize(telemetryEvent, JsonOptions); + + lock (_lock) + { + try + { + var line = _contextPrefix + ": " + json + Environment.NewLine; + File.AppendAllText(_filePath, line); + } + catch + { + // Ignore file write errors in telemetry to avoid breaking the application + } + } + } + } +} diff --git a/src/Uno.DevTools.Telemetry/HashBuilder.cs b/src/Uno.DevTools.Telemetry/HashBuilder.cs index 6fccf05..1b07ece 100644 --- a/src/Uno.DevTools.Telemetry/HashBuilder.cs +++ b/src/Uno.DevTools.Telemetry/HashBuilder.cs @@ -1,9 +1,5 @@ -using System; -using System.Linq; -using System.Runtime.InteropServices; -using System.Security.Cryptography; +using System.Security.Cryptography; using System.Text; -using Microsoft.CodeAnalysis; namespace Uno.DevTools.Telemetry.Helpers { @@ -17,29 +13,26 @@ internal class HashBuilder /// to limit collisions for duplicate file names in different folders, used in the same generator. /// /// The string to hash - /// The algorithm to use /// A hex-formatted string of the hash public static string Build(string input) { - using (var algorithm = SHA256.Create()) - { - var data = algorithm.ComputeHash(Encoding.UTF8.GetBytes(input)); - - var min = Math.Min(data.Length, 16); + using var algorithm = SHA256.Create(); + var data = algorithm.ComputeHash(Encoding.UTF8.GetBytes(input)); - Span span = stackalloc char[32]; - int i = 0, j = 0; - while (i < min) - { - var b = data[i++]; - var nib = (b >> 4); - span[j++] = nib > 9 ? (char)(nib + 'W') : (char)(nib + '0'); - nib = (b & 0xf); - span[j++] = nib > 9 ? (char)(nib + 'W') : (char)(nib + '0'); - } + var min = Math.Min(data.Length, 16); - return span.Slice(0, min * 2).ToString(); + Span span = stackalloc char[32]; + int i = 0, j = 0; + while (i < min) + { + var b = data[i++]; + var nib = (b >> 4); + span[j++] = nib > 9 ? (char)(nib + 'W') : (char)(nib + '0'); + nib = (b & 0xf); + span[j++] = nib > 9 ? (char)(nib + 'W') : (char)(nib + '0'); } + + return span.Slice(0, min * 2).ToString(); } } } diff --git a/src/Uno.DevTools.Telemetry/TelemetryServiceCollectionExtensions.cs b/src/Uno.DevTools.Telemetry/TelemetryServiceCollectionExtensions.cs new file mode 100644 index 0000000..979f65e --- /dev/null +++ b/src/Uno.DevTools.Telemetry/TelemetryServiceCollectionExtensions.cs @@ -0,0 +1,52 @@ +using Microsoft.Extensions.DependencyInjection; +using System.Reflection; +using System; + +namespace Uno.DevTools.Telemetry +{ + public static class TelemetryServiceCollectionExtensions + { + /// + /// Adds Uno.DevTools.Telemetry to the service collection. + /// + /// The service collection. + /// The Application Insights instrumentation key. + /// Event name prefix (context). + /// Optional assembly for version info. If null, uses calling assembly. + /// Optional session id. + /// The service collection. + public static IServiceCollection AddTelemetry( + this IServiceCollection services, + string instrumentationKey, + string eventNamePrefix, + Assembly? versionAssembly = null, + string? sessionId = null) + { + versionAssembly ??= Assembly.GetCallingAssembly(); + + // Check for file-based telemetry override + var filePath = Environment.GetEnvironmentVariable("UNO_PLATFORM_TELEMETRY_FILE"); + if (!string.IsNullOrEmpty(filePath)) + { + services.AddSingleton(sp => + { +#if NET8_0_OR_GREATER + var timeProvider = sp.GetService(); + return new FileTelemetry(filePath, eventNamePrefix, timeProvider); +#else + return new FileTelemetry(filePath, eventNamePrefix); +#endif + }); + } + else + { + services.AddSingleton(sp => new Telemetry( + instrumentationKey, + eventNamePrefix, + versionAssembly, + sessionId)); + } + return services; + } + } +} diff --git a/src/Uno.DevTools.Telemetry/Uno.DevTools.Telemetry.csproj b/src/Uno.DevTools.Telemetry/Uno.DevTools.Telemetry.csproj index 0793255..2d889d5 100644 --- a/src/Uno.DevTools.Telemetry/Uno.DevTools.Telemetry.csproj +++ b/src/Uno.DevTools.Telemetry/Uno.DevTools.Telemetry.csproj @@ -7,13 +7,12 @@ Uno.DevTools.Telemetry - 0.1.0-dev.1 Uno Platform Copyright (c) Uno Platform 2015-$([System.DateTime]::Now.ToString(`yyyy`)) A development-time dependency for telemetry true true - + LICENSE.md @@ -24,10 +23,15 @@ + all runtime; build; native; contentfiles; analyzers; buildtransitive + + + + From 2acfaac32a5f05e5f740e4cc496edca115a10d58 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Mon, 7 Jul 2025 15:23:45 -0400 Subject: [PATCH 4/8] ci(test): modernize CI and add MSTest unit tests - Update CI workflow for MSTest 3.8.3 and TRX reporting - Add Uno.DevTools.Telemetry.Tests project with FileTelemetry unit tests - Centralize package versions (CPVM) and update solution/props files --- .github/workflows/ci.yml | 36 ++++++++++--- src/Directory.Build.props | 2 +- src/Directory.Packages.props | 2 + .../FileTelemetryTests.cs | 51 +++++++++++++++++++ .../Uno.DevTools.Telemetry.Tests.csproj | 14 +++++ src/Uno.DevTools.Telemetry.sln | 28 +++++++++- 6 files changed, 125 insertions(+), 8 deletions(-) create mode 100644 src/Uno.DevTools.Telemetry.Tests/FileTelemetryTests.cs create mode 100644 src/Uno.DevTools.Telemetry.Tests/Uno.DevTools.Telemetry.Tests.csproj diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5212511..809165b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,7 +18,7 @@ jobs: runs-on: windows-2022 steps: - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v4 with: fetch-depth: 0 @@ -38,6 +38,30 @@ jobs: name: NuGet path: .\artifacts + - name: Create Test Results Directory + shell: pwsh + run: New-Item -ItemType Directory -Force -Path artifacts/test-results + + - name: Test + shell: pwsh + run: | + cd src + dotnet run --project Uno.DevTools.Telemetry.Tests/Uno.DevTools.Telemetry.Tests.csproj --configuration Release -- --report-trx --results-directory ../artifacts/test-results + + - name: Upload Test Results + uses: actions/upload-artifact@v4 + with: + name: test-results + path: artifacts/test-results/*.trx + + - name: Publish Test Results + uses: dorny/test-reporter@v2 + if: always() + with: + name: Unit Tests + path: artifacts/test-results/*.trx + reporter: dotnet-trx + sign: name: Sign Package if: ${{ github.event_name == 'push' && (github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/heads/release/')) }} @@ -51,7 +75,7 @@ jobs: - build steps: - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v4 - name: Download Artifacts uses: actions/download-artifact@v4 @@ -60,14 +84,14 @@ jobs: path: artifacts\NuGet - name: Setup .NET SDK - uses: actions/setup-dotnet@v1 + uses: actions/setup-dotnet@v4 with: dotnet-version: '9.0.x' - name: Setup SignClient run: dotnet tool install --tool-path . sign --version 0.9.1-beta.25278.1 - # Login to Azure using a ServicePrincipal configured to authenticate agaist a GitHub Action + # Login to Azure using a ServicePrincipal configured to authenticate against a GitHub Action - name: 'Az CLI login' uses: azure/login@v1 with: @@ -83,8 +107,8 @@ jobs: ./sign code azure-key-vault artifacts\NuGet\*.nupkg --publisher-name "Uno.Devtools.Telemetry" - --description "Uno.Devtools.Telemetryk" --description-url "https://github.com/${{ env.GITHUB_REPOSITORY }}" + --description "Uno.Devtools.Telemetry" --azure-key-vault-managed-identity true --azure-key-vault-url "${{ secrets.SIGN_KEY_VAULT_URL }}" --azure-key-vault-certificate "${{ secrets.SIGN_KEY_VAULT_CERTIFICATE_ID }}" @@ -136,4 +160,4 @@ jobs: - name: NuGet Push shell: pwsh run: | - dotnet nuget push artifacts\*.nupkg -s https://api.nuget.org/v3/index.json -k "${{ secrets.NUGET_ORG_API_KEY }}" \ No newline at end of file + dotnet nuget push artifacts\*.nupkg -s https://api.nuget.org/v3/index.json -k "${{ secrets.NUGET_ORG_API_KEY }}" diff --git a/src/Directory.Build.props b/src/Directory.Build.props index 4d4c138..480735b 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -4,7 +4,7 @@ enable true true - $(NoWarn);NU1507 + $(NoWarn);NU1507;NU5030 true true diff --git a/src/Directory.Packages.props b/src/Directory.Packages.props index 8f0fa16..51d3c6e 100644 --- a/src/Directory.Packages.props +++ b/src/Directory.Packages.props @@ -6,6 +6,8 @@ + + \ No newline at end of file diff --git a/src/Uno.DevTools.Telemetry.Tests/FileTelemetryTests.cs b/src/Uno.DevTools.Telemetry.Tests/FileTelemetryTests.cs new file mode 100644 index 0000000..d2e34f6 --- /dev/null +++ b/src/Uno.DevTools.Telemetry.Tests/FileTelemetryTests.cs @@ -0,0 +1,51 @@ +using AwesomeAssertions; +using System; +using System.Collections.Generic; +using System.IO; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Uno.DevTools.Telemetry.Tests +{ + [TestClass] + public class FileTelemetryTests + { + private string GetTempFilePath() => Path.Combine(Path.GetTempPath(), $"telemetry_test_{Guid.NewGuid():N}.log"); + + [TestMethod] + public void TrackEvent_WritesToFile() + { + var filePath = GetTempFilePath(); + var telemetry = new FileTelemetry(filePath, "test"); + + telemetry.TrackEvent("TestEvent", new Dictionary { { "foo", "bar" } }, null); + + telemetry.Flush(); + + var lines = File.ReadAllLines(filePath); + lines.Should().HaveCount(1); + lines[0].Should().Contain("TestEvent"); + lines[0].Should().Contain("foo"); + lines[0].Should().Contain("bar"); + } + + [TestMethod] + public void TrackEvent_MultipleEvents_AreAllWritten() + { + var filePath = GetTempFilePath(); + var telemetry = new FileTelemetry(filePath, "multi"); + + for (var i = 0; i < 5; i++) + { + telemetry.TrackEvent($"Event{i}", (IDictionary?)null, (IDictionary?)null); + } + telemetry.Flush(); + + var lines = File.ReadAllLines(filePath); + lines.Should().HaveCount(5); + for (var i = 0; i < 5; i++) + { + lines[i].Should().Contain($"Event{i}"); + } + } + } +} diff --git a/src/Uno.DevTools.Telemetry.Tests/Uno.DevTools.Telemetry.Tests.csproj b/src/Uno.DevTools.Telemetry.Tests/Uno.DevTools.Telemetry.Tests.csproj new file mode 100644 index 0000000..aad18c5 --- /dev/null +++ b/src/Uno.DevTools.Telemetry.Tests/Uno.DevTools.Telemetry.Tests.csproj @@ -0,0 +1,14 @@ + + + net8.0 + false + + + + + + + + + + diff --git a/src/Uno.DevTools.Telemetry.sln b/src/Uno.DevTools.Telemetry.sln index b19a75d..b07855a 100644 --- a/src/Uno.DevTools.Telemetry.sln +++ b/src/Uno.DevTools.Telemetry.sln @@ -1,7 +1,7 @@  Microsoft Visual Studio Solution File, Format Version 12.00 # Visual Studio Version 17 -VisualStudioVersion = 17.13.35507.96 d17.13 +VisualStudioVersion = 17.13.35507.96 MinimumVisualStudioVersion = 10.0.40219.1 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Uno.DevTools.Telemetry", "Uno.DevTools.Telemetry\Uno.DevTools.Telemetry.csproj", "{291C2E10-5CAA-465B-A14E-CB7954AB25A8}" EndProject @@ -15,16 +15,42 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Files", "Files", "{A7C5A7B2 ..\.github\workflows\ci.yml = ..\.github\workflows\ci.yml EndProjectSection EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Uno.DevTools.Telemetry.Tests", "Uno.DevTools.Telemetry.Tests\Uno.DevTools.Telemetry.Tests.csproj", "{14DF6269-8857-4B00-B93A-79E58BC0B7B0}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU + Debug|x64 = Debug|x64 + Debug|x86 = Debug|x86 Release|Any CPU = Release|Any CPU + Release|x64 = Release|x64 + Release|x86 = Release|x86 EndGlobalSection GlobalSection(ProjectConfigurationPlatforms) = postSolution {291C2E10-5CAA-465B-A14E-CB7954AB25A8}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {291C2E10-5CAA-465B-A14E-CB7954AB25A8}.Debug|Any CPU.Build.0 = Debug|Any CPU + {291C2E10-5CAA-465B-A14E-CB7954AB25A8}.Debug|x64.ActiveCfg = Debug|Any CPU + {291C2E10-5CAA-465B-A14E-CB7954AB25A8}.Debug|x64.Build.0 = Debug|Any CPU + {291C2E10-5CAA-465B-A14E-CB7954AB25A8}.Debug|x86.ActiveCfg = Debug|Any CPU + {291C2E10-5CAA-465B-A14E-CB7954AB25A8}.Debug|x86.Build.0 = Debug|Any CPU {291C2E10-5CAA-465B-A14E-CB7954AB25A8}.Release|Any CPU.ActiveCfg = Release|Any CPU {291C2E10-5CAA-465B-A14E-CB7954AB25A8}.Release|Any CPU.Build.0 = Release|Any CPU + {291C2E10-5CAA-465B-A14E-CB7954AB25A8}.Release|x64.ActiveCfg = Release|Any CPU + {291C2E10-5CAA-465B-A14E-CB7954AB25A8}.Release|x64.Build.0 = Release|Any CPU + {291C2E10-5CAA-465B-A14E-CB7954AB25A8}.Release|x86.ActiveCfg = Release|Any CPU + {291C2E10-5CAA-465B-A14E-CB7954AB25A8}.Release|x86.Build.0 = Release|Any CPU + {14DF6269-8857-4B00-B93A-79E58BC0B7B0}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {14DF6269-8857-4B00-B93A-79E58BC0B7B0}.Debug|Any CPU.Build.0 = Debug|Any CPU + {14DF6269-8857-4B00-B93A-79E58BC0B7B0}.Debug|x64.ActiveCfg = Debug|Any CPU + {14DF6269-8857-4B00-B93A-79E58BC0B7B0}.Debug|x64.Build.0 = Debug|Any CPU + {14DF6269-8857-4B00-B93A-79E58BC0B7B0}.Debug|x86.ActiveCfg = Debug|Any CPU + {14DF6269-8857-4B00-B93A-79E58BC0B7B0}.Debug|x86.Build.0 = Debug|Any CPU + {14DF6269-8857-4B00-B93A-79E58BC0B7B0}.Release|Any CPU.ActiveCfg = Release|Any CPU + {14DF6269-8857-4B00-B93A-79E58BC0B7B0}.Release|Any CPU.Build.0 = Release|Any CPU + {14DF6269-8857-4B00-B93A-79E58BC0B7B0}.Release|x64.ActiveCfg = Release|Any CPU + {14DF6269-8857-4B00-B93A-79E58BC0B7B0}.Release|x64.Build.0 = Release|Any CPU + {14DF6269-8857-4B00-B93A-79E58BC0B7B0}.Release|x86.ActiveCfg = Release|Any CPU + {14DF6269-8857-4B00-B93A-79E58BC0B7B0}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE From f299b47347ab5d247e278854b1210e670e41c742 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Mon, 7 Jul 2025 15:24:33 -0400 Subject: [PATCH 5/8] refactor: apply var, readonly, and style fixes for immutability and clarity - Use var and readonly where appropriate in PersistenceChannel and Telemetry - Improve thread-safety and immutability in Telemetry event chaining - Minor style and doc fixes --- .../PersistenceChannel/FlushManager.cs | 4 +- .../PersistenceChannel/PersistenceChannel.cs | 2 +- .../PersistenceTransmitter.cs | 6 +- .../PersistenceChannel/Sender.cs | 14 ++-- .../SnapshottingCollection.cs | 4 +- .../SnapshottingDictionary.cs | 2 +- .../PersistenceChannel/StorageService.cs | 34 ++++----- .../PersistenceChannel/StorageTransmission.cs | 24 +++---- src/Uno.DevTools.Telemetry/Telemetry.cs | 71 ++++++++++++------- .../TelemetryCommonProperties.cs | 4 +- 10 files changed, 93 insertions(+), 72 deletions(-) diff --git a/src/Uno.DevTools.Telemetry/PersistenceChannel/FlushManager.cs b/src/Uno.DevTools.Telemetry/PersistenceChannel/FlushManager.cs index 62968ba..6caa662 100644 --- a/src/Uno.DevTools.Telemetry/PersistenceChannel/FlushManager.cs +++ b/src/Uno.DevTools.Telemetry/PersistenceChannel/FlushManager.cs @@ -50,8 +50,8 @@ internal void Flush(IChannelTelemetry telemetryItem) { if (telemetryItem != null) { - byte[] data = JsonSerializer.Serialize(new[] { telemetryItem }); - Transmission transmission = new Transmission( + var data = JsonSerializer.Serialize(new[] { telemetryItem }); + var transmission = new Transmission( EndpointAddress, data, "application/x-json-stream", diff --git a/src/Uno.DevTools.Telemetry/PersistenceChannel/PersistenceChannel.cs b/src/Uno.DevTools.Telemetry/PersistenceChannel/PersistenceChannel.cs index 3b9c5a5..971230c 100644 --- a/src/Uno.DevTools.Telemetry/PersistenceChannel/PersistenceChannel.cs +++ b/src/Uno.DevTools.Telemetry/PersistenceChannel/PersistenceChannel.cs @@ -87,7 +87,7 @@ public string? EndpointAddress set { - string address = value ?? TelemetryServiceEndpoint; + var address = value ?? TelemetryServiceEndpoint; _flushManager.EndpointAddress = new Uri(address); } } diff --git a/src/Uno.DevTools.Telemetry/PersistenceChannel/PersistenceTransmitter.cs b/src/Uno.DevTools.Telemetry/PersistenceChannel/PersistenceTransmitter.cs index dda0972..1c53b4b 100644 --- a/src/Uno.DevTools.Telemetry/PersistenceChannel/PersistenceTransmitter.cs +++ b/src/Uno.DevTools.Telemetry/PersistenceChannel/PersistenceTransmitter.cs @@ -48,7 +48,7 @@ internal PersistenceTransmitter(BaseStorageService storage, int sendersCount, bo _storage = storage; if (createSenders) { - for (int i = 0; i < sendersCount; i++) + for (var i = 0; i < sendersCount; i++) { _senders.Add(new Sender(_storage, this)); } @@ -82,8 +82,8 @@ private void StopSenders() return; } - List stoppedTasks = new List(); - foreach (Sender sender in _senders) + var stoppedTasks = new List(); + foreach (var sender in _senders) { stoppedTasks.Add(sender.StopAsync()); } diff --git a/src/Uno.DevTools.Telemetry/PersistenceChannel/Sender.cs b/src/Uno.DevTools.Telemetry/PersistenceChannel/Sender.cs index 0173d1e..d4b5f68 100644 --- a/src/Uno.DevTools.Telemetry/PersistenceChannel/Sender.cs +++ b/src/Uno.DevTools.Telemetry/PersistenceChannel/Sender.cs @@ -171,13 +171,13 @@ internal Task StopAsync() /// protected void SendLoop() { - TimeSpan prevSendingInterval = TimeSpan.Zero; - TimeSpan sendingInterval = _sendingIntervalOnNoData; + var prevSendingInterval = TimeSpan.Zero; + var sendingInterval = _sendingIntervalOnNoData; try { while (!_stopped) { - using (StorageTransmission? transmission = _storage.Peek()) + using (var transmission = _storage.Peek()) { if (_stopped) { @@ -190,7 +190,7 @@ protected void SendLoop() // If there is a transmission to send - send it. if (transmission != null) { - bool shouldRetry = Send(transmission, ref sendingInterval); + var shouldRetry = Send(transmission, ref sendingInterval); if (!shouldRetry) { // If retry is not required - delete the transmission. @@ -230,7 +230,7 @@ protected virtual bool Send(StorageTransmission transmission, ref TimeSpan nextS { if (transmission != null) { - bool isConnected = NetworkInterface.GetIsNetworkAvailable(); + var isConnected = NetworkInterface.GetIsNetworkAvailable(); // there is no internet connection available, return than. if (!isConnected) @@ -248,7 +248,7 @@ protected virtual bool Send(StorageTransmission transmission, ref TimeSpan nextS } catch (WebException e) { - int? statusCode = GetStatusCode(e); + var statusCode = GetStatusCode(e); nextSendInterval = CalculateNextInterval(statusCode, nextSendInterval, _maxIntervalBetweenRetries); return IsRetryable(statusCode, e.Status); } @@ -337,7 +337,7 @@ private TimeSpan CalculateNextInterval(int? httpStatusCode, TimeSpan currentSend return TimeSpan.FromSeconds(1); } - double nextIntervalInSeconds = Math.Min(currentSendInterval.TotalSeconds * 2, maxInterval.TotalSeconds); + var nextIntervalInSeconds = Math.Min(currentSendInterval.TotalSeconds * 2, maxInterval.TotalSeconds); return TimeSpan.FromSeconds(nextIntervalInSeconds); } diff --git a/src/Uno.DevTools.Telemetry/PersistenceChannel/SnapshottingCollection.cs b/src/Uno.DevTools.Telemetry/PersistenceChannel/SnapshottingCollection.cs index 35973e4..4be69e4 100644 --- a/src/Uno.DevTools.Telemetry/PersistenceChannel/SnapshottingCollection.cs +++ b/src/Uno.DevTools.Telemetry/PersistenceChannel/SnapshottingCollection.cs @@ -60,7 +60,7 @@ public bool Remove(TItem item) { lock (Collection) { - bool removed = Collection.Remove(item); + var removed = Collection.Remove(item); if (removed) { snapshot = default; @@ -84,7 +84,7 @@ IEnumerator IEnumerable.GetEnumerator() protected TCollection GetSnapshot() { - TCollection? localSnapshot = snapshot; + var localSnapshot = snapshot; if (localSnapshot == null) { lock (Collection) diff --git a/src/Uno.DevTools.Telemetry/PersistenceChannel/SnapshottingDictionary.cs b/src/Uno.DevTools.Telemetry/PersistenceChannel/SnapshottingDictionary.cs index e4f7e76..35df727 100644 --- a/src/Uno.DevTools.Telemetry/PersistenceChannel/SnapshottingDictionary.cs +++ b/src/Uno.DevTools.Telemetry/PersistenceChannel/SnapshottingDictionary.cs @@ -55,7 +55,7 @@ public bool Remove(TKey key) { lock (Collection) { - bool removed = Collection.Remove(key); + var removed = Collection.Remove(key); if (removed) { snapshot = null; diff --git a/src/Uno.DevTools.Telemetry/PersistenceChannel/StorageService.cs b/src/Uno.DevTools.Telemetry/PersistenceChannel/StorageService.cs index c89f293..a0f7ffd 100644 --- a/src/Uno.DevTools.Telemetry/PersistenceChannel/StorageService.cs +++ b/src/Uno.DevTools.Telemetry/PersistenceChannel/StorageService.cs @@ -120,13 +120,13 @@ private void VerifyOrSetDefaultStorageDirectoryPath(string? desireStorageDirecto /// internal override StorageTransmission? Peek() { - IEnumerable files = GetFiles("*.trn", 50); + var files = GetFiles("*.trn", 50); if (PeekedTransmissions is not null) { lock (_peekLockObj) { - foreach (string file in files) + foreach (var file in files) { try { @@ -135,7 +135,7 @@ private void VerifyOrSetDefaultStorageDirectoryPath(string? desireStorageDirecto _deletedFilesQueue.Contains(file) == false) { // Load the transmission from disk. - StorageTransmission storageTransmissionItem = LoadTransmissionFromFileAsync(file) + var storageTransmissionItem = LoadTransmissionFromFileAsync(file) .ConfigureAwait(false).GetAwaiter().GetResult(); // when item is disposed it should be removed from the peeked list. @@ -172,7 +172,7 @@ internal override void Delete(StorageTransmission item) // Initial storage size calculation. CalculateSize(); - long fileSize = GetSize(item.FileName); + var fileSize = GetSize(item.FileName); File.Delete(Path.Combine(StorageFolder, item.FileName)); _deletedFilesQueue.Enqueue(item.FileName); @@ -212,7 +212,7 @@ internal override async Task EnqueueAsync(Transmission transmission) // Writes content to a temporary file and only then rename to avoid the Peek from reading the file before it is being written. // Creates the temp file name - string tempFileName = Guid.NewGuid().ToString("N"); + var tempFileName = Guid.NewGuid().ToString("N"); // Now that the file got created we can increase the files count Interlocked.Increment(ref _storageCountFiles); @@ -221,12 +221,12 @@ internal override async Task EnqueueAsync(Transmission transmission) await SaveTransmissionToFileAsync(transmission, tempFileName).ConfigureAwait(false); // Now that the file is written increase storage size. - long temporaryFileSize = GetSize(tempFileName); + var temporaryFileSize = GetSize(tempFileName); Interlocked.Add(ref _storageSize, temporaryFileSize); // Creates a new file name - string now = DateTime.UtcNow.ToString("yyyyMMddHHmmss", CultureInfo.InvariantCulture); - string newFileName = string.Format(CultureInfo.InvariantCulture, "{0}_{1}.trn", now, tempFileName); + var now = DateTime.UtcNow.ToString("yyyyMMddHHmmss", CultureInfo.InvariantCulture); + var newFileName = string.Format(CultureInfo.InvariantCulture, "{0}_{1}.trn", now, tempFileName); // Renames the file File.Move(Path.Combine(StorageFolder, tempFileName), Path.Combine(StorageFolder, newFileName)); @@ -253,7 +253,7 @@ private async Task SaveTransmissionToFileAsync(Transmission transmission, string } catch (UnauthorizedAccessException) { - string message = + var message = string.Format( CultureInfo.InvariantCulture, "Failed to save transmission to file. UnauthorizedAccessException. File path: {0}, FileName: {1}", @@ -275,14 +275,14 @@ private async Task LoadTransmissionFromFileAsync(string fil using (Stream stream = File.OpenRead(Path.Combine(StorageFolder, file))) { - StorageTransmission storageTransmissionItem = + var storageTransmissionItem = await StorageTransmission.CreateFromStreamAsync(stream, file).ConfigureAwait(false); return storageTransmissionItem; } } catch (Exception e) { - string message = + var message = string.Format( CultureInfo.InvariantCulture, "Failed to load transmission from file. File path: {0}, FileName: {1}, Exception: {2}", @@ -325,7 +325,7 @@ private long GetSize(string file) { if (StorageFolder is not null) { - using (FileStream stream = File.OpenRead(Path.Combine(StorageFolder, file))) + using (var stream = File.OpenRead(Path.Combine(StorageFolder, file))) { return stream.Length; } @@ -344,12 +344,12 @@ private void CalculateSize() { if (StorageFolder is not null) { - string[] storageFiles = Directory.GetFiles(StorageFolder, "*.*"); + var storageFiles = Directory.GetFiles(StorageFolder, "*.*"); _storageCountFiles = storageFiles.Length; long storageSizeInBytes = 0; - foreach (string file in storageFiles) + foreach (var file in storageFiles) { storageSizeInBytes += GetSize(file); } @@ -372,10 +372,10 @@ private void DeleteObsoleteFiles() { if (StorageFolder is not null) { - IEnumerable files = GetFiles("*.tmp", 50); - foreach (string file in files) + var files = GetFiles("*.tmp", 50); + foreach (var file in files) { - DateTime creationTime = File.GetCreationTimeUtc(Path.Combine(StorageFolder, file)); + var creationTime = File.GetCreationTimeUtc(Path.Combine(StorageFolder, file)); // if the file is older then 5 minutes - delete it. if (DateTime.UtcNow - creationTime >= TimeSpan.FromMinutes(5)) { diff --git a/src/Uno.DevTools.Telemetry/PersistenceChannel/StorageTransmission.cs b/src/Uno.DevTools.Telemetry/PersistenceChannel/StorageTransmission.cs index 4513e13..d5ca08d 100644 --- a/src/Uno.DevTools.Telemetry/PersistenceChannel/StorageTransmission.cs +++ b/src/Uno.DevTools.Telemetry/PersistenceChannel/StorageTransmission.cs @@ -46,11 +46,11 @@ public void Dispose() /// Return transmission loaded from file; return null if the file is corrupted. internal static async Task CreateFromStreamAsync(Stream stream, string fileName) { - StreamReader reader = new StreamReader(stream); - Uri address = await ReadAddressAsync(reader).ConfigureAwait(false); - string contentType = await ReadHeaderAsync(reader, "Content-Type").ConfigureAwait(false); - string contentEncoding = await ReadHeaderAsync(reader, "Content-Encoding").ConfigureAwait(false); - byte[] content = await ReadContentAsync(reader).ConfigureAwait(false); + var reader = new StreamReader(stream); + var address = await ReadAddressAsync(reader).ConfigureAwait(false); + var contentType = await ReadHeaderAsync(reader, "Content-Type").ConfigureAwait(false); + var contentEncoding = await ReadHeaderAsync(reader, "Content-Encoding").ConfigureAwait(false); + var content = await ReadContentAsync(reader).ConfigureAwait(false); return new StorageTransmission(fileName, address, content, contentType, contentEncoding); } @@ -59,7 +59,7 @@ internal static async Task CreateFromStreamAsync(Stream str /// internal static async Task SaveAsync(Transmission transmission, Stream stream) { - StreamWriter writer = new StreamWriter(stream); + var writer = new StreamWriter(stream); try { await writer.WriteLineAsync(transmission.EndpointAddress.ToString()).ConfigureAwait(false); @@ -77,14 +77,14 @@ await writer.WriteLineAsync("Content-Encoding" + ":" + transmission.ContentEncod private static async Task ReadHeaderAsync(TextReader reader, string headerName) { - string? line = await reader.ReadLineAsync().ConfigureAwait(false); + var line = await reader.ReadLineAsync().ConfigureAwait(false); if (string.IsNullOrEmpty(line)) { throw new FormatException(string.Format(CultureInfo.InvariantCulture, "{0} header is expected.", headerName)); } - string[] parts = line.Split(':'); + var parts = line.Split(':'); if (parts.Length != 2) { throw new FormatException(string.Format(CultureInfo.InvariantCulture, @@ -102,19 +102,19 @@ private static async Task ReadHeaderAsync(TextReader reader, string head private static async Task ReadAddressAsync(TextReader reader) { - string? addressLine = await reader.ReadLineAsync().ConfigureAwait(false); + var addressLine = await reader.ReadLineAsync().ConfigureAwait(false); if (string.IsNullOrEmpty(addressLine)) { throw new FormatException("Transmission address is expected."); } - Uri address = new Uri(addressLine); + var address = new Uri(addressLine); return address; } private static async Task ReadContentAsync(TextReader reader) { - string? content = await reader.ReadToEndAsync().ConfigureAwait(false); + var content = await reader.ReadToEndAsync().ConfigureAwait(false); if (string.IsNullOrEmpty(content) || content == Environment.NewLine) { throw new FormatException("Content is expected."); @@ -127,7 +127,7 @@ private void Dispose(bool disposing) { if (disposing) { - Action? disposingDelegate = Disposing; + var disposingDelegate = Disposing; disposingDelegate?.Invoke(this); } } diff --git a/src/Uno.DevTools.Telemetry/Telemetry.cs b/src/Uno.DevTools.Telemetry/Telemetry.cs index e146468..1e24c1a 100644 --- a/src/Uno.DevTools.Telemetry/Telemetry.cs +++ b/src/Uno.DevTools.Telemetry/Telemetry.cs @@ -17,17 +17,21 @@ namespace Uno.DevTools.Telemetry { public class Telemetry : ITelemetry { - private string? _currentSessionId; + private readonly string? _currentSessionId; private TelemetryClient? _client; private Dictionary? _commonProperties; private Dictionary? _commonMeasurements; private TelemetryConfiguration? _telemetryConfig; private Task? _trackEventTask; + // _trackEventTask is updated using a lock-free CAS loop (see TrackEvent) + // to ensure that all telemetry events are chained in order, even under heavy concurrency. + // This avoids the need for a global lock and guarantees that no events are lost or overwritten. + private readonly object _trackEventTaskLock = new(); private string? _storageDirectoryPath; private string? _settingsStorageDirectoryPath; private PersistenceChannel.PersistenceChannel? _persistenceChannel; - private string _instrumentationKey; - private string _eventNamePrefix; + private readonly string _instrumentationKey; + private readonly string _eventNamePrefix; private readonly Assembly _versionAssembly; private readonly string? _productName; private readonly Func? _currentDirectoryProvider; @@ -39,11 +43,12 @@ public class Telemetry : ITelemetry /// Initializes a new instance of the class. /// /// The App Insights Key + /// A prefix that will be used on all events through this telemetry instance + /// The assembly to use to get the version to report in telemetry + /// Defines the session ID for this instance + /// Block the execution of the constructor until the telemetry is initialized /// A delegate that can determine if the telemetry is enabled /// A delegate that can provide the value to be hashed in the "Current Path Hash" custom dimension - /// Block the execution of the constructor until the telemetry is initialized - /// Defines the session ID for this instance - /// The assembly to use to get the version to report in telemetry /// The product name to use in the common properties. If null, versionAssembly.Name is used instead. public Telemetry( string instrumentationKey, @@ -84,7 +89,7 @@ public Telemetry( else { //initialize in task to offload to parallel thread - _trackEventTask = Task.Run(() => InitializeTelemetry()); + _trackEventTask = Task.Run(InitializeTelemetry); } } @@ -102,10 +107,24 @@ public void TrackEvent(string eventName, IDictionary? properties return; } - //continue the task in different threads - _trackEventTask = _trackEventTask.ContinueWith( - x => TrackEventTask(eventName, properties, measurements) - ); + // Lock-free chaining of telemetry events: + // 1. Read the current task (originalTask) + // 2. Create a continuation that will send the new event after originalTask + // 3. Atomically swap _trackEventTask to the new continuation only if it still matches originalTask + // 4. If another thread changed it, retry with the new value + // This ensures all events are sent in order, even with concurrent calls. + while (true) + { + var originalTask = _trackEventTask; + var continuation = originalTask.ContinueWith( + x => TrackEventTask(eventName, properties, measurements) + ); + var exchanged = Interlocked.CompareExchange(ref _trackEventTask, continuation, originalTask); + if (exchanged == originalTask) + { + break; + } + } } public void Flush() @@ -115,10 +134,12 @@ public void Flush() return; } - // Skip the wait if the task has not yet been activated - if (_trackEventTask.Status != TaskStatus.WaitingForActivation) + // Wait for the current chain of telemetry events to complete. + // This reads the value atomically and does not block other threads from chaining new events. + var task = _trackEventTask; + if (task.Status != TaskStatus.WaitingForActivation) { - _trackEventTask.Wait(TimeSpan.FromSeconds(1)); + task.Wait(TimeSpan.FromSeconds(1)); } } @@ -129,9 +150,11 @@ public async Task FlushAsync(CancellationToken ct) return; } - if (!_trackEventTask.IsCompleted) + // Wait asynchronously for the current chain of telemetry events to complete. + var task = _trackEventTask; + if (!task.IsCompleted) { - await Task.WhenAny(_trackEventTask, Task.Delay(-1, ct)); + await Task.WhenAny(task, Task.Delay(-1, ct)); } } @@ -242,19 +265,17 @@ private Dictionary GetEventMeasures(IDictionary? private Dictionary? GetEventProperties(IDictionary? properties) { - if (properties != null) + if (properties == null) { - var eventProperties = new Dictionary(_commonProperties ?? []); - foreach (var property in properties) - { - eventProperties[property.Key] = property.Value; - } - return eventProperties; + return _commonProperties; } - else + + var eventProperties = new Dictionary(_commonProperties ?? []); + foreach (var property in properties) { - return _commonProperties; + eventProperties[property.Key] = property.Value; } + return eventProperties; } } } diff --git a/src/Uno.DevTools.Telemetry/TelemetryCommonProperties.cs b/src/Uno.DevTools.Telemetry/TelemetryCommonProperties.cs index 5b8dfdd..ad35723 100644 --- a/src/Uno.DevTools.Telemetry/TelemetryCommonProperties.cs +++ b/src/Uno.DevTools.Telemetry/TelemetryCommonProperties.cs @@ -40,8 +40,8 @@ public TelemetryCommonProperties( _productName = productName; } - private Func _getCurrentDirectory; - private string _storageDirectoryPath; + private readonly Func _getCurrentDirectory; + private readonly string _storageDirectoryPath; private readonly Assembly _versionAssembly; private readonly string _productName; From d9ad320d6e7495681cb212a403cba95c35236b31 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Mon, 7 Jul 2025 15:58:49 -0400 Subject: [PATCH 6/8] feat(telemetry): improve concurrency, IO robustness, and test coverage - Documented collection immutability and thread-safety in Telemetry and BaseStorageService. - Improved FileTelemetry IO error handling and logging. - Refactored Sender to use Interlocked for lockless stop logic. - Added multi-threaded stress test for FileTelemetry. - Updated CI/test infra and project metadata. - Updated README with collection mutation warning. --- README.md | 4 ++ src/Directory.Build.props | 3 ++ .../FileTelemetryTests.cs | 51 +++++++++++++++++-- .../GlobalUsings.cs | 6 +++ src/Uno.DevTools.Telemetry/FileTelemetry.cs | 15 ++++-- .../PersistenceChannel/BaseStorageService.cs | 2 + .../PersistenceChannel/Sender.cs | 10 ++-- src/Uno.DevTools.Telemetry/Telemetry.cs | 2 + .../Uno.DevTools.Telemetry.csproj | 9 ++-- 9 files changed, 87 insertions(+), 15 deletions(-) create mode 100644 src/Uno.DevTools.Telemetry.Tests/GlobalUsings.cs diff --git a/README.md b/README.md index 0f16e02..fdb1798 100644 --- a/README.md +++ b/README.md @@ -60,6 +60,10 @@ telemetry.TrackEvent("AppStarted"); telemetry.TrackEvent("UserAction", new Dictionary { { "Action", "Clicked" } }, null); ``` +> **Warning:** +> When using `ITelemetry` (including `FileTelemetry` or any implementation), do not modify any dictionary or list after passing it as a parameter to telemetry methods (such as `TrackEvent`). +> All collections passed to telemetry should be considered owned by the telemetry system and must not be mutated by the caller after the call. Mutating collections after passing them may cause race conditions or undefined behavior. + ### File-based Telemetry By default, telemetry is persisted locally before being sent. You can configure the storage location and behavior by customizing the `Telemetry` constructor. diff --git a/src/Directory.Build.props b/src/Directory.Build.props index 480735b..75b19f2 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -11,6 +11,9 @@ true true snupkg + + Uno Platform + Copyright (c) Uno Platform 2015-$([System.DateTime]::Now.ToString(`yyyy`)) diff --git a/src/Uno.DevTools.Telemetry.Tests/FileTelemetryTests.cs b/src/Uno.DevTools.Telemetry.Tests/FileTelemetryTests.cs index d2e34f6..a4cdeec 100644 --- a/src/Uno.DevTools.Telemetry.Tests/FileTelemetryTests.cs +++ b/src/Uno.DevTools.Telemetry.Tests/FileTelemetryTests.cs @@ -1,8 +1,6 @@ -using AwesomeAssertions; -using System; using System.Collections.Generic; using System.IO; -using Microsoft.VisualStudio.TestTools.UnitTesting; +using System.Threading; namespace Uno.DevTools.Telemetry.Tests { @@ -47,5 +45,52 @@ public void TrackEvent_MultipleEvents_AreAllWritten() lines[i].Should().Contain($"Event{i}"); } } + + [TestMethod] + public void TrackEvent_MultiThreaded_StressTest() + { + var filePath = GetTempFilePath(); + var telemetry = new FileTelemetry(filePath, "stress"); + var threadCount = 16; + var eventsPerThread = 800; + var totalEvents = threadCount * eventsPerThread; + var threads = new List(); + var exceptions = new List(); + var startEvent = new ManualResetEventSlim(false); + + for (var t = 0; t < threadCount; t++) + { + threads.Add(new Thread(() => + { + try + { + startEvent.Wait(); // Ensure all threads start together + for (var i = 0; i < eventsPerThread; i++) + { + telemetry.TrackEvent($"StressEvent", new Dictionary { { "thread", Thread.CurrentThread.ManagedThreadId.ToString() }, { "i", i.ToString() } }, null); + } + } + catch (Exception ex) + { + lock (exceptions) { exceptions.Add(ex); } + } + })); + } + + threads.ForEach(t => t.Start()); + startEvent.Set(); + threads.ForEach(t => t.Join()); + telemetry.Flush(); + + exceptions.Should().BeEmpty(); + var lines = File.ReadAllLines(filePath); + lines.Should().HaveCount(totalEvents); + foreach (var line in lines) + { + line.Should().Contain("StressEvent"); + line.Should().Contain("thread"); + line.Should().Contain("i"); + } + } } } diff --git a/src/Uno.DevTools.Telemetry.Tests/GlobalUsings.cs b/src/Uno.DevTools.Telemetry.Tests/GlobalUsings.cs new file mode 100644 index 0000000..3036e69 --- /dev/null +++ b/src/Uno.DevTools.Telemetry.Tests/GlobalUsings.cs @@ -0,0 +1,6 @@ +// Global using directives + +global using System; +global using AwesomeAssertions; +global using Microsoft.VisualStudio.TestTools.UnitTesting; +global using Uno.DevTools.Telemetry.PersistenceChannel; \ No newline at end of file diff --git a/src/Uno.DevTools.Telemetry/FileTelemetry.cs b/src/Uno.DevTools.Telemetry/FileTelemetry.cs index 90ab124..02ed990 100644 --- a/src/Uno.DevTools.Telemetry/FileTelemetry.cs +++ b/src/Uno.DevTools.Telemetry/FileTelemetry.cs @@ -55,7 +55,15 @@ private static void EnsureDirectoryExists(string filePath) var directory = Path.GetDirectoryName(filePath); if (!string.IsNullOrEmpty(directory) && !Directory.Exists(directory)) { - Directory.CreateDirectory(directory); + try + { + Directory.CreateDirectory(directory); + } + catch (Exception ex) + { + // Log the exception for diagnostics (but do not throw) + System.Diagnostics.Debug.WriteLine($"[FileTelemetry] Failed to create directory '{directory}': {ex}"); + } } } @@ -132,9 +140,10 @@ public void TrackEvent(string eventName, IDictionary? properties var line = _contextPrefix + ": " + json + Environment.NewLine; File.AppendAllText(_filePath, line); } - catch + catch (Exception ex) { - // Ignore file write errors in telemetry to avoid breaking the application + // Log the exception for diagnostics (but do not throw) + System.Diagnostics.Debug.WriteLine($"[FileTelemetry] Failed to write telemetry to '{_filePath}': {ex}"); } } } diff --git a/src/Uno.DevTools.Telemetry/PersistenceChannel/BaseStorageService.cs b/src/Uno.DevTools.Telemetry/PersistenceChannel/BaseStorageService.cs index e8378a0..110afd1 100644 --- a/src/Uno.DevTools.Telemetry/PersistenceChannel/BaseStorageService.cs +++ b/src/Uno.DevTools.Telemetry/PersistenceChannel/BaseStorageService.cs @@ -18,6 +18,8 @@ internal abstract class BaseStorageService { /// /// Peeked transmissions dictionary (maps file name to its full path). Holds all the transmissions that were peeked. + /// + /// IMPORTANT: Internal collections must never be exposed directly. All access and mutation must go through thread-safe abstractions (e.g., SnapshottingDictionary). /// /// /// Note: The value (=file's full path) is not required in the Storage implementation. diff --git a/src/Uno.DevTools.Telemetry/PersistenceChannel/Sender.cs b/src/Uno.DevTools.Telemetry/PersistenceChannel/Sender.cs index d4b5f68..5f1454e 100644 --- a/src/Uno.DevTools.Telemetry/PersistenceChannel/Sender.cs +++ b/src/Uno.DevTools.Telemetry/PersistenceChannel/Sender.cs @@ -63,7 +63,7 @@ internal class Sender : IDisposable /// A boolean value that indicates if the sender should be stopped. The sender's while loop is checking this boolean /// value. /// - private bool _stopped; + private int _stopped; /// /// The transmissions storage. @@ -90,7 +90,7 @@ internal class Sender : IDisposable /// internal Sender(BaseStorageService storage, PersistenceTransmitter transmitter, bool startSending = true) { - _stopped = false; + _stopped = 0; DelayHandler = new AutoResetEvent(false); _stoppedHandler = new AutoResetEvent(false); _drainingTimeout = TimeSpan.FromSeconds(100); @@ -149,7 +149,7 @@ internal Task StopAsync() { // After delayHandler is set, a sending iteration will immediately start. // Setting stopped to true, will cause the iteration to skip the actual sending and stop immediately. - _stopped = true; + Interlocked.Exchange(ref _stopped, 1); DelayHandler.Set(); // if delayHandler was set while a transmission was being sent, the return task will wait for it to finish, for an additional second, @@ -175,11 +175,11 @@ protected void SendLoop() var sendingInterval = _sendingIntervalOnNoData; try { - while (!_stopped) + while (Interlocked.CompareExchange(ref _stopped, 0, 0) == 0) { using (var transmission = _storage.Peek()) { - if (_stopped) + if (Interlocked.CompareExchange(ref _stopped, 0, 0) != 0) { // This second verification is required for cases where 'stopped' was set while peek was happening. // Once the actual sending starts the design is to wait until it finishes and deletes the transmission. diff --git a/src/Uno.DevTools.Telemetry/Telemetry.cs b/src/Uno.DevTools.Telemetry/Telemetry.cs index 1e24c1a..23df110 100644 --- a/src/Uno.DevTools.Telemetry/Telemetry.cs +++ b/src/Uno.DevTools.Telemetry/Telemetry.cs @@ -19,6 +19,8 @@ public class Telemetry : ITelemetry { private readonly string? _currentSessionId; private TelemetryClient? _client; + // These collections must be treated as immutable after construction. + // Do not mutate after initialization to avoid race conditions in concurrent scenarios. private Dictionary? _commonProperties; private Dictionary? _commonMeasurements; private TelemetryConfiguration? _telemetryConfig; diff --git a/src/Uno.DevTools.Telemetry/Uno.DevTools.Telemetry.csproj b/src/Uno.DevTools.Telemetry/Uno.DevTools.Telemetry.csproj index 2d889d5..98c3ff7 100644 --- a/src/Uno.DevTools.Telemetry/Uno.DevTools.Telemetry.csproj +++ b/src/Uno.DevTools.Telemetry/Uno.DevTools.Telemetry.csproj @@ -6,11 +6,12 @@ - Uno.DevTools.Telemetry - Uno Platform - Copyright (c) Uno Platform 2015-$([System.DateTime]::Now.ToString(`yyyy`)) A development-time dependency for telemetry true + + + + Uno.DevTools.Telemetry true LICENSE.md @@ -19,7 +20,7 @@ True sn.snk - + From 0474b84e7ea811e2d29f355bf983418adc651977 Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Mon, 7 Jul 2025 16:20:46 -0400 Subject: [PATCH 7/8] feat(telemetry): add ITelemetry interface, DI support, and improve test coverage - Introduce ITelemetry and TelemetryFactory for contextual/typed telemetry - Add TelemetryAdapter and DI support (AddTelemetry, AddTelemetry, AddTelemetryGenericSupport) - Add and refactor unit tests for ITelemetry and FileTelemetry (Given/When/Then, AAA) - Update README with usage, DI, and concurrency warnings - Improve concurrency, IO error handling, and documentation - Fix CPVM and centralize NuGet dependencies --- README.md | 33 ++++++ src/Directory.Packages.props | 1 + .../FileTelemetryTests.cs | 16 ++- .../GlobalUsings.cs | 3 +- .../TelemetryGenericDiTests.cs | 46 ++++++++ .../Uno.DevTools.Telemetry.Tests.csproj | 1 + src/Uno.DevTools.Telemetry/ITelemetryT.cs | 45 ++++++++ .../TelemetryAdapter.cs | 45 ++++++++ .../TelemetryAttribute.cs | 29 +++++ .../TelemetryServiceCollectionExtensions.cs | 106 ++++++++++++------ 10 files changed, 283 insertions(+), 42 deletions(-) create mode 100644 src/Uno.DevTools.Telemetry.Tests/TelemetryGenericDiTests.cs create mode 100644 src/Uno.DevTools.Telemetry/ITelemetryT.cs create mode 100644 src/Uno.DevTools.Telemetry/TelemetryAdapter.cs create mode 100644 src/Uno.DevTools.Telemetry/TelemetryAttribute.cs diff --git a/README.md b/README.md index fdb1798..5e894bc 100644 --- a/README.md +++ b/README.md @@ -116,3 +116,36 @@ Replace `path/to/YourApp.csproj` with the path to your application's project fil > **Tip:** You can also specify an absolute path for the log file (e.g., `C:\temp\telemetry.log`). +## Advanced usage: typed/contextual telemetry with DI + +To inject contextualized telemetry by type: + +```csharp +// In your Startup or Program.cs +services.AddTelemetry(); + +// Somewhere in your assembly containing the service +[assembly: Telemetry("instrumentation-key", "prefix")] + +// In an application class +public class MyService +{ + private readonly ITelemetry _telemetry; + public MyService(ITelemetry telemetry) // Telemetry will be properly configured using the [assembly: Telemetry] attribute + { + _telemetry = telemetry; + } + public void DoSomething() + { + _telemetry.TrackEvent("Action", new Dictionary { { "key", "value" } }, null); + } +} +``` + +The injected instance will automatically use the assembly-level configuration of `MyService` (the `[Telemetry]` attribute). + +- If the `UNO_PLATFORM_TELEMETRY_FILE` environment variable is set, the instance will be a `FileTelemetry`. +- Otherwise, the instance will be a `Telemetry` (Application Insights). + +> **Note:** You can inject `ITelemetry` for any type, and resolution will be automatic via the DI container. + diff --git a/src/Directory.Packages.props b/src/Directory.Packages.props index 51d3c6e..9d54a35 100644 --- a/src/Directory.Packages.props +++ b/src/Directory.Packages.props @@ -6,6 +6,7 @@ + diff --git a/src/Uno.DevTools.Telemetry.Tests/FileTelemetryTests.cs b/src/Uno.DevTools.Telemetry.Tests/FileTelemetryTests.cs index a4cdeec..1d8ba03 100644 --- a/src/Uno.DevTools.Telemetry.Tests/FileTelemetryTests.cs +++ b/src/Uno.DevTools.Telemetry.Tests/FileTelemetryTests.cs @@ -10,15 +10,17 @@ public class FileTelemetryTests private string GetTempFilePath() => Path.Combine(Path.GetTempPath(), $"telemetry_test_{Guid.NewGuid():N}.log"); [TestMethod] - public void TrackEvent_WritesToFile() + public void Given_FileTelemetry_When_TrackEvent_Then_WritesToFile() { + // Arrange var filePath = GetTempFilePath(); var telemetry = new FileTelemetry(filePath, "test"); + // Act telemetry.TrackEvent("TestEvent", new Dictionary { { "foo", "bar" } }, null); - telemetry.Flush(); + // Assert var lines = File.ReadAllLines(filePath); lines.Should().HaveCount(1); lines[0].Should().Contain("TestEvent"); @@ -27,17 +29,20 @@ public void TrackEvent_WritesToFile() } [TestMethod] - public void TrackEvent_MultipleEvents_AreAllWritten() + public void Given_FileTelemetry_When_TrackEvent_MultipleEvents_Then_AllAreWritten() { + // Arrange var filePath = GetTempFilePath(); var telemetry = new FileTelemetry(filePath, "multi"); + // Act for (var i = 0; i < 5; i++) { telemetry.TrackEvent($"Event{i}", (IDictionary?)null, (IDictionary?)null); } telemetry.Flush(); + // Assert var lines = File.ReadAllLines(filePath); lines.Should().HaveCount(5); for (var i = 0; i < 5; i++) @@ -47,8 +52,9 @@ public void TrackEvent_MultipleEvents_AreAllWritten() } [TestMethod] - public void TrackEvent_MultiThreaded_StressTest() + public void Given_FileTelemetry_When_TrackEvent_MultiThreaded_Then_AllEventsAreWrittenWithoutError() { + // Arrange var filePath = GetTempFilePath(); var telemetry = new FileTelemetry(filePath, "stress"); var threadCount = 16; @@ -77,11 +83,13 @@ public void TrackEvent_MultiThreaded_StressTest() })); } + // Act threads.ForEach(t => t.Start()); startEvent.Set(); threads.ForEach(t => t.Join()); telemetry.Flush(); + // Assert exceptions.Should().BeEmpty(); var lines = File.ReadAllLines(filePath); lines.Should().HaveCount(totalEvents); diff --git a/src/Uno.DevTools.Telemetry.Tests/GlobalUsings.cs b/src/Uno.DevTools.Telemetry.Tests/GlobalUsings.cs index 3036e69..4eaa126 100644 --- a/src/Uno.DevTools.Telemetry.Tests/GlobalUsings.cs +++ b/src/Uno.DevTools.Telemetry.Tests/GlobalUsings.cs @@ -3,4 +3,5 @@ global using System; global using AwesomeAssertions; global using Microsoft.VisualStudio.TestTools.UnitTesting; -global using Uno.DevTools.Telemetry.PersistenceChannel; \ No newline at end of file +global using Uno.DevTools.Telemetry.PersistenceChannel; +global using Microsoft.Extensions.DependencyInjection; \ No newline at end of file diff --git a/src/Uno.DevTools.Telemetry.Tests/TelemetryGenericDiTests.cs b/src/Uno.DevTools.Telemetry.Tests/TelemetryGenericDiTests.cs new file mode 100644 index 0000000..13d6d2d --- /dev/null +++ b/src/Uno.DevTools.Telemetry.Tests/TelemetryGenericDiTests.cs @@ -0,0 +1,46 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +[assembly: Uno.DevTools.Telemetry.Telemetry("test-key", EventsPrefix = "test-prefix")] // For test context + +namespace Uno.DevTools.Telemetry.Tests; + +[TestClass] +public class TelemetryGenericDiTests +{ + public class MyContext {} + + [TestMethod] + public void Given_TelemetryGenericDiTests_When_ITelemetryT_FileTelemetry_Writes_To_File_Then_Content_Is_Valid() + { + // Arrange + var tempFile = Path.GetTempFileName(); + Environment.SetEnvironmentVariable("UNO_PLATFORM_TELEMETRY_FILE", tempFile); + try + { + IServiceCollection services = new ServiceCollection(); + services.AddTelemetry(); + var provider = services.BuildServiceProvider(); + var telemetry = provider.GetRequiredService>(); + + // Act + telemetry.TrackEvent("TestEvent", new Dictionary { { "foo", "bar" } }, null); + telemetry.Flush(); + + // Assert + var lines = File.ReadAllLines(tempFile).Where(l => !string.IsNullOrWhiteSpace(l)).ToArray(); + Assert.AreEqual(1, lines.Length, "Should have exactly one telemetry event written"); + StringAssert.Contains(lines[0], "TestEvent"); + StringAssert.Contains(lines[0], "foo"); + StringAssert.Contains(lines[0], "bar"); + } + finally + { + Environment.SetEnvironmentVariable("UNO_PLATFORM_TELEMETRY_FILE", null); + } + } +} diff --git a/src/Uno.DevTools.Telemetry.Tests/Uno.DevTools.Telemetry.Tests.csproj b/src/Uno.DevTools.Telemetry.Tests/Uno.DevTools.Telemetry.Tests.csproj index aad18c5..75c23e6 100644 --- a/src/Uno.DevTools.Telemetry.Tests/Uno.DevTools.Telemetry.Tests.csproj +++ b/src/Uno.DevTools.Telemetry.Tests/Uno.DevTools.Telemetry.Tests.csproj @@ -10,5 +10,6 @@ + diff --git a/src/Uno.DevTools.Telemetry/ITelemetryT.cs b/src/Uno.DevTools.Telemetry/ITelemetryT.cs new file mode 100644 index 0000000..30f488f --- /dev/null +++ b/src/Uno.DevTools.Telemetry/ITelemetryT.cs @@ -0,0 +1,45 @@ +using System; +using System.Reflection; + +namespace Uno.DevTools.Telemetry +{ + /// + /// Generic interface for contextual/typed telemetry. + /// + public interface ITelemetry : ITelemetry { } + + + /// + /// Factory for ITelemetry that resolves assembly-level configuration and instantiates the correct telemetry implementation. + /// + public static class TelemetryFactory + { + /// + /// Creates a contextual ITelemetry instance, using the TelemetryAttribute on the assembly of T. + /// + public static ITelemetry Create() + { + var assembly = typeof(T).Assembly; + var attr = assembly.GetCustomAttribute(); + if (attr == null) + throw new InvalidOperationException($"Assembly {assembly.GetName().Name} is missing [Telemetry] attribute."); + + var instrumentationKey = attr.InstrumentationKey; + var prefix = attr.EventsPrefix ?? string.Empty; + + // File-based telemetry override + var filePath = Environment.GetEnvironmentVariable("UNO_PLATFORM_TELEMETRY_FILE"); + if (!string.IsNullOrEmpty(filePath)) + { +#if NET8_0_OR_GREATER + return new TelemetryAdapter(new FileTelemetry(filePath, prefix)); +#else + return new TelemetryAdapter(new FileTelemetry(filePath, prefix)); +#endif + } + + // Default: Application Insights + return new TelemetryAdapter(new Telemetry(instrumentationKey, prefix, assembly)); + } + } +} diff --git a/src/Uno.DevTools.Telemetry/TelemetryAdapter.cs b/src/Uno.DevTools.Telemetry/TelemetryAdapter.cs new file mode 100644 index 0000000..e89b697 --- /dev/null +++ b/src/Uno.DevTools.Telemetry/TelemetryAdapter.cs @@ -0,0 +1,45 @@ +using Microsoft.Extensions.DependencyInjection; + +namespace Uno.DevTools.Telemetry; + +public record TelemetryAdapter : ITelemetry +{ + private readonly ITelemetry Inner; + + public TelemetryAdapter(ITelemetry inner) + { + Inner = inner; + } + + public TelemetryAdapter(IServiceProvider services) + { + Inner = services.GetService()!; + } + + /// + public void Dispose() + => Inner.Dispose(); + + /// + public void Flush() + => Inner.Flush(); + + /// + public Task FlushAsync(CancellationToken ct) + => Inner.FlushAsync(ct); + + /// + public void ThreadBlockingTrackEvent(string eventName, IDictionary properties, IDictionary measurements) + => Inner.ThreadBlockingTrackEvent(eventName, properties, measurements); + + /// + public void TrackEvent(string eventName, (string key, string value)[]? properties, (string key, double value)[]? measurements) + => Inner.TrackEvent(eventName, properties, measurements); + + /// + public void TrackEvent(string eventName, IDictionary? properties, IDictionary? measurements) + => Inner.TrackEvent(eventName, properties, measurements); + + /// + public bool Enabled => Inner.Enabled; +} \ No newline at end of file diff --git a/src/Uno.DevTools.Telemetry/TelemetryAttribute.cs b/src/Uno.DevTools.Telemetry/TelemetryAttribute.cs new file mode 100644 index 0000000..1af3020 --- /dev/null +++ b/src/Uno.DevTools.Telemetry/TelemetryAttribute.cs @@ -0,0 +1,29 @@ +namespace Uno.DevTools.Telemetry; + +/// +/// Configures telemetry for an assembly. This attribute must be applied at the assembly level +/// for telemetry to work properly. +/// +/// +/// The telemetry system uses this attribute to configure how events are tracked and reported. +/// When the telemetry system is initialized, it looks for this attribute on the assembly +/// to determine the instrumentation key and event prefix to use. +/// +/// Example usage: +/// [assembly: Telemetry("MyApp", EventsPrefix = "myapp/module")] +/// +[AttributeUsage(AttributeTargets.Assembly)] +public class TelemetryAttribute(string instrumentationKey) : Attribute +{ + /// + /// Gets the instrumentation key used to identify the telemetry source. + /// This key is used to route telemetry data to the appropriate destination. + /// + public string InstrumentationKey { get; } = instrumentationKey; + + /// + /// Gets or sets the prefix to apply to all event names generated from this assembly. + /// If not specified, a default prefix based on the assembly name will be used. + /// + public string? EventsPrefix { get; set; } = string.Empty; +} \ No newline at end of file diff --git a/src/Uno.DevTools.Telemetry/TelemetryServiceCollectionExtensions.cs b/src/Uno.DevTools.Telemetry/TelemetryServiceCollectionExtensions.cs index 979f65e..b4d24ce 100644 --- a/src/Uno.DevTools.Telemetry/TelemetryServiceCollectionExtensions.cs +++ b/src/Uno.DevTools.Telemetry/TelemetryServiceCollectionExtensions.cs @@ -2,51 +2,83 @@ using System.Reflection; using System; -namespace Uno.DevTools.Telemetry +namespace Uno.DevTools.Telemetry; + +public static class TelemetryServiceCollectionExtensions { - public static class TelemetryServiceCollectionExtensions + /// + /// Adds Uno.DevTools.Telemetry to the service collection. + /// + /// The service collection. + /// The Application Insights instrumentation key. + /// Event name prefix (context). + /// Optional assembly for version info. If null, uses calling assembly. + /// Optional session id. + /// The service collection. + public static IServiceCollection AddTelemetry( + this IServiceCollection services, + string instrumentationKey, + string eventNamePrefix, + Assembly? versionAssembly = null, + string? sessionId = null) { - /// - /// Adds Uno.DevTools.Telemetry to the service collection. - /// - /// The service collection. - /// The Application Insights instrumentation key. - /// Event name prefix (context). - /// Optional assembly for version info. If null, uses calling assembly. - /// Optional session id. - /// The service collection. - public static IServiceCollection AddTelemetry( - this IServiceCollection services, - string instrumentationKey, - string eventNamePrefix, - Assembly? versionAssembly = null, - string? sessionId = null) - { - versionAssembly ??= Assembly.GetCallingAssembly(); + versionAssembly ??= Assembly.GetCallingAssembly(); - // Check for file-based telemetry override - var filePath = Environment.GetEnvironmentVariable("UNO_PLATFORM_TELEMETRY_FILE"); - if (!string.IsNullOrEmpty(filePath)) + // Check for file-based telemetry override + var filePath = Environment.GetEnvironmentVariable("UNO_PLATFORM_TELEMETRY_FILE"); + if (!string.IsNullOrEmpty(filePath)) + { + services.AddSingleton(sp => { - services.AddSingleton(sp => - { #if NET8_0_OR_GREATER var timeProvider = sp.GetService(); return new FileTelemetry(filePath, eventNamePrefix, timeProvider); #else - return new FileTelemetry(filePath, eventNamePrefix); + return new FileTelemetry(filePath, eventNamePrefix); #endif - }); - } - else - { - services.AddSingleton(sp => new Telemetry( - instrumentationKey, - eventNamePrefix, - versionAssembly, - sessionId)); - } - return services; + }); } + else + { + services.AddSingleton(sp => new Telemetry( + instrumentationKey, + eventNamePrefix, + versionAssembly, + sessionId)); + } + return services; + } + + /// + /// Adds Uno.DevTools.Telemetry for a generic context type to the service collection. + /// + /// The context type for telemetry (usually your main class). + /// The service collection. + /// The service collection. + public static IServiceCollection AddTelemetry(this IServiceCollection services) + { + services.AddSingleton>(sp => TelemetryFactory.Create()); + return services; + } + + /// + /// Registers ITelemetry for all T in the DI container, using assembly-level configuration. + /// + public static IServiceCollection AddTelemetry(this IServiceCollection services) + { + services.AddTransient(typeof(ITelemetry<>), typeof(TelemetryGenericFactory<>)); + return services; + } + + private class TelemetryGenericFactory(IServiceProvider sp) : ITelemetry + { + private readonly ITelemetry _inner = TelemetryFactory.Create(); + public bool Enabled => _inner.Enabled; + public void Dispose() => _inner.Dispose(); + public void Flush() => _inner.Flush(); + public Task FlushAsync(CancellationToken ct) => _inner.FlushAsync(ct); + public void ThreadBlockingTrackEvent(string eventName, IDictionary properties, IDictionary measurements) => _inner.ThreadBlockingTrackEvent(eventName, properties, measurements); + public void TrackEvent(string eventName, (string key, string value)[]? properties, (string key, double value)[]? measurements) => _inner.TrackEvent(eventName, properties, measurements); + public void TrackEvent(string eventName, IDictionary? properties, IDictionary? measurements) => _inner.TrackEvent(eventName, properties, measurements); } -} +} \ No newline at end of file From 98ca465e142f82f19469aa8a427b94384274986e Mon Sep 17 00:00:00 2001 From: Carl de Billy Date: Mon, 7 Jul 2025 16:26:43 -0400 Subject: [PATCH 8/8] chore: mark sealed classes and test refactor as done in todos - Marked all possible classes as sealed - Updated todos to reflect test refactor to Given/When/Then and AwesomeAssertions --- .../TelemetryGenericDiTests.cs | 16 +++++----------- src/Uno.DevTools.Telemetry/FileTelemetry.cs | 2 +- .../PersistenceChannel/FixedSizeQueue.cs | 2 +- .../PersistenceChannel/FlushManager.cs | 2 +- .../PersistenceChannel/PersistenceTransmitter.cs | 2 +- .../PersistenceChannel/Sender.cs | 8 ++++---- src/Uno.DevTools.Telemetry/Telemetry.cs | 2 +- src/Uno.DevTools.Telemetry/TelemetryAttribute.cs | 2 +- .../TelemetryCommonProperties.cs | 2 +- 9 files changed, 16 insertions(+), 22 deletions(-) diff --git a/src/Uno.DevTools.Telemetry.Tests/TelemetryGenericDiTests.cs b/src/Uno.DevTools.Telemetry.Tests/TelemetryGenericDiTests.cs index 13d6d2d..a4cbcba 100644 --- a/src/Uno.DevTools.Telemetry.Tests/TelemetryGenericDiTests.cs +++ b/src/Uno.DevTools.Telemetry.Tests/TelemetryGenericDiTests.cs @@ -1,10 +1,3 @@ -using System; -using System.Collections.Generic; -using System.IO; -using System.Linq; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.VisualStudio.TestTools.UnitTesting; - [assembly: Uno.DevTools.Telemetry.Telemetry("test-key", EventsPrefix = "test-prefix")] // For test context namespace Uno.DevTools.Telemetry.Tests; @@ -33,10 +26,11 @@ public void Given_TelemetryGenericDiTests_When_ITelemetryT_FileTelemetry_Writes_ // Assert var lines = File.ReadAllLines(tempFile).Where(l => !string.IsNullOrWhiteSpace(l)).ToArray(); - Assert.AreEqual(1, lines.Length, "Should have exactly one telemetry event written"); - StringAssert.Contains(lines[0], "TestEvent"); - StringAssert.Contains(lines[0], "foo"); - StringAssert.Contains(lines[0], "bar"); + lines.Should().HaveCount(1, "Should have exactly one telemetry event written"); + lines[0].Should().Contain("TestEvent"); + lines[0].Should().Contain("foo"); + lines[0].Should().Contain("bar"); + lines[0].Should().Contain("test-prefix"); } finally { diff --git a/src/Uno.DevTools.Telemetry/FileTelemetry.cs b/src/Uno.DevTools.Telemetry/FileTelemetry.cs index 02ed990..fa2a46e 100644 --- a/src/Uno.DevTools.Telemetry/FileTelemetry.cs +++ b/src/Uno.DevTools.Telemetry/FileTelemetry.cs @@ -11,7 +11,7 @@ namespace Uno.DevTools.Telemetry /// A telemetry implementation that writes events to a file for testing purposes. /// Can use either individual files per context or a single file with prefixes. /// - public class FileTelemetry : ITelemetry + public sealed class FileTelemetry : ITelemetry { private static readonly JsonSerializerOptions JsonOptions = new() { diff --git a/src/Uno.DevTools.Telemetry/PersistenceChannel/FixedSizeQueue.cs b/src/Uno.DevTools.Telemetry/PersistenceChannel/FixedSizeQueue.cs index a5b4c69..15a4830 100644 --- a/src/Uno.DevTools.Telemetry/PersistenceChannel/FixedSizeQueue.cs +++ b/src/Uno.DevTools.Telemetry/PersistenceChannel/FixedSizeQueue.cs @@ -15,7 +15,7 @@ namespace Uno.DevTools.Telemetry.PersistenceChannel /// A light fixed size queue. If Enqueue is called and queue's limit has reached the last item will be removed. /// This data structure is thread safe. /// - internal class FixedSizeQueue + internal sealed class FixedSizeQueue { private readonly int _maxSize; private readonly Queue _queue = new Queue(); diff --git a/src/Uno.DevTools.Telemetry/PersistenceChannel/FlushManager.cs b/src/Uno.DevTools.Telemetry/PersistenceChannel/FlushManager.cs index 6caa662..1ff860e 100644 --- a/src/Uno.DevTools.Telemetry/PersistenceChannel/FlushManager.cs +++ b/src/Uno.DevTools.Telemetry/PersistenceChannel/FlushManager.cs @@ -17,7 +17,7 @@ namespace Uno.DevTools.Telemetry.PersistenceChannel /// /// This class handles all the logic for flushing the In Memory buffer to the persistent storage. /// - internal class FlushManager + internal sealed class FlushManager { /// /// The storage that is used to persist all the transmissions. diff --git a/src/Uno.DevTools.Telemetry/PersistenceChannel/PersistenceTransmitter.cs b/src/Uno.DevTools.Telemetry/PersistenceChannel/PersistenceTransmitter.cs index 1c53b4b..5e83440 100644 --- a/src/Uno.DevTools.Telemetry/PersistenceChannel/PersistenceTransmitter.cs +++ b/src/Uno.DevTools.Telemetry/PersistenceChannel/PersistenceTransmitter.cs @@ -17,7 +17,7 @@ namespace Uno.DevTools.Telemetry.PersistenceChannel /// /// Implements throttled and persisted transmission of telemetry to Application Insights. /// - internal class PersistenceTransmitter : IDisposable + internal sealed class PersistenceTransmitter : IDisposable { /// /// The number of times this object was disposed. diff --git a/src/Uno.DevTools.Telemetry/PersistenceChannel/Sender.cs b/src/Uno.DevTools.Telemetry/PersistenceChannel/Sender.cs index 5f1454e..74d2e92 100644 --- a/src/Uno.DevTools.Telemetry/PersistenceChannel/Sender.cs +++ b/src/Uno.DevTools.Telemetry/PersistenceChannel/Sender.cs @@ -19,7 +19,7 @@ namespace Uno.DevTools.Telemetry.PersistenceChannel /// /// Fetch transmissions from the storage and sends it. /// - internal class Sender : IDisposable + internal sealed class Sender : IDisposable { /// /// The default sending interval. @@ -29,7 +29,7 @@ internal class Sender : IDisposable /// /// A wait handle that flags the sender when to start sending again. The type is protected for unit test. /// - protected readonly AutoResetEvent DelayHandler; + private readonly AutoResetEvent DelayHandler; /// /// Holds the maximum time for the exponential back-off algorithm. The sending interval will grow on every HTTP @@ -169,7 +169,7 @@ internal Task StopAsync() /// /// Send transmissions in a loop. /// - protected void SendLoop() + private void SendLoop() { var prevSendingInterval = TimeSpan.Zero; var sendingInterval = _sendingIntervalOnNoData; @@ -224,7 +224,7 @@ protected void SendLoop() /// iteration. /// /// True, if there was sent error and we need to retry sending, otherwise false. - protected virtual bool Send(StorageTransmission transmission, ref TimeSpan nextSendInterval) + private bool Send(StorageTransmission transmission, ref TimeSpan nextSendInterval) { try { diff --git a/src/Uno.DevTools.Telemetry/Telemetry.cs b/src/Uno.DevTools.Telemetry/Telemetry.cs index 23df110..9d32261 100644 --- a/src/Uno.DevTools.Telemetry/Telemetry.cs +++ b/src/Uno.DevTools.Telemetry/Telemetry.cs @@ -15,7 +15,7 @@ namespace Uno.DevTools.Telemetry { - public class Telemetry : ITelemetry + public sealed class Telemetry : ITelemetry { private readonly string? _currentSessionId; private TelemetryClient? _client; diff --git a/src/Uno.DevTools.Telemetry/TelemetryAttribute.cs b/src/Uno.DevTools.Telemetry/TelemetryAttribute.cs index 1af3020..b2c8e44 100644 --- a/src/Uno.DevTools.Telemetry/TelemetryAttribute.cs +++ b/src/Uno.DevTools.Telemetry/TelemetryAttribute.cs @@ -13,7 +13,7 @@ namespace Uno.DevTools.Telemetry; /// [assembly: Telemetry("MyApp", EventsPrefix = "myapp/module")] /// [AttributeUsage(AttributeTargets.Assembly)] -public class TelemetryAttribute(string instrumentationKey) : Attribute +public sealed class TelemetryAttribute(string instrumentationKey) : Attribute { /// /// Gets the instrumentation key used to identify the telemetry source. diff --git a/src/Uno.DevTools.Telemetry/TelemetryCommonProperties.cs b/src/Uno.DevTools.Telemetry/TelemetryCommonProperties.cs index ad35723..41c66eb 100644 --- a/src/Uno.DevTools.Telemetry/TelemetryCommonProperties.cs +++ b/src/Uno.DevTools.Telemetry/TelemetryCommonProperties.cs @@ -26,7 +26,7 @@ namespace Uno.DevTools.Telemetry { - internal class TelemetryCommonProperties + internal sealed class TelemetryCommonProperties { public TelemetryCommonProperties( string storageDirectoryPath,