-
-
Notifications
You must be signed in to change notification settings - Fork 254
Enable Open Telemetry in bit Boilerplate windows & maui clients (#11682) #11683
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe changes migrate client-side telemetry from Application Insights to OpenTelemetry for MAUI and Windows platforms, introduce a Changes
Sequence DiagramsequenceDiagram
participant App as MAUI/Windows App
participant OTel as OpenTelemetry
participant AzMon as Azure Monitor Exporter
participant OTLP as OTLP Exporter
rect rgb(200, 220, 240)
Note over App,OTLP: Old Flow (Application Insights)
App->>App: ITelemetryInitializer.Initialize()
App->>App: Populate telemetry context
App->>App: Send to Application Insights
end
rect rgb(220, 240, 220)
Note over App,OTLP: New Flow (OpenTelemetry)
App->>OTel: AddOpenTelemetry.AddLogging()
OTel->>OTel: IncludeFormattedMessage & IncludeScopes
alt Azure Monitor Exporter
OTel->>AzMon: Azure Monitor exporter<br/>(if connection string)
AzMon->>App: Telemetry exported
end
alt OTLP Exporter
OTel->>OTLP: OTLP exporter<br/>(if OTEL_EXPORTER_OTLP_ENDPOINT)
OTLP->>App: Telemetry exported
end
end
sequenceDiagram
participant DevHost as AppHost
participant Windows as Windows Client
participant Maui as MAUI Client
participant Devices as Platform Devices
rect rgb(240, 220, 220)
Note over DevHost,Devices: AppHost Multi-Platform Setup
DevHost->>DevHost: Check OS.IsWindows()
alt Windows
DevHost->>Windows: Add Windows project
Windows->>Devices: Configure Windows app
end
DevHost->>Maui: Add MAUI project
alt Platform Conditionals
Maui->>Devices: Windows device
Maui->>Devices: macOS (MacCatalyst)
Maui->>Devices: iOS device + simulator
Maui->>Devices: Android device + emulator
end
Devices->>Devices: Enable OTLP tunnels<br/>(iOS devices)
Devices->>DevHost: Connect to AppHost tunnels
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enables OpenTelemetry support for Windows and MAUI clients in the bit Boilerplate template, migrating from direct Application Insights integration to a unified OpenTelemetry-based approach with support for both OTLP and Azure Monitor exporters.
Key Changes:
- Replaces Application Insights telemetry initializers with OpenTelemetry processors for client applications
- Adds Aspire MAUI hosting support with platform-specific device configurations
- Updates Sentry packages to rc.1 and replaces Application Insights logging with Azure Monitor OpenTelemetry exporter
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| Boilerplate.Server.AppHost/Program.cs | Adds MAUI project configuration with OS-specific device runners (Windows, Mac Catalyst, iOS, Android) and OTLP dev tunnel support; moves Windows client to OS-conditional block |
| Boilerplate.Server.AppHost.csproj | Adds Aspire.Hosting.Maui package reference for MAUI project hosting support |
| Boilerplate.Server.Api/SignalR/AppChatbot.cs | Renames variables to variablesPrompt for improved clarity |
| Directory.Packages.props | Updates Sentry packages to 6.0.0-rc.1, adds Aspire.Hosting.Maui and Microsoft.Extensions.Configuration.EnvironmentVariables, replaces Application Insights logging with Azure.Monitor.OpenTelemetry.Exporter |
| Boilerplate.Client.Windows/Services/WindowsTelemetryEnrichmentProcessor.cs | New OpenTelemetry processor that enriches telemetry with Windows-specific context and filters Blazor SignalR/event activities |
| Boilerplate.Client.Windows/Services/WindowsAppInsightsTelemetryInitializer.cs | Removed - replaced by OpenTelemetry-based enrichment |
| Boilerplate.Client.Windows/Program.cs | Adds environment variables configuration support for OpenTelemetry endpoint injection |
| Boilerplate.Client.Windows/Program.Services.cs | Replaces Application Insights logging with OpenTelemetry configuration |
| Boilerplate.Client.Windows/Boilerplate.Client.Windows.csproj | Updates package references for OpenTelemetry support, links shared extension file |
| Boilerplate.Client.Maui/Services/MauiTelemetryEnrichmentProcessor.cs | New OpenTelemetry processor with MAUI-specific device enrichment and activity filtering |
| Boilerplate.Client.Maui/Services/MauiAppInsightsTelemetryInitializer.cs | Removed - replaced by OpenTelemetry-based enrichment |
| Boilerplate.Client.Maui/MauiProgram.Services.cs | Replaces Application Insights logging with OpenTelemetry configuration |
| Boilerplate.Client.Maui/Extensions/IOpenTelemetryExtensions.cs | New extension method for configuring OpenTelemetry with OTLP and Azure Monitor exporters |
| Boilerplate.Client.Maui/Boilerplate.Client.Maui.csproj | Adds OpenTelemetry and Azure Monitor package references |
Comments suppressed due to low confidence (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props:62
- Inconsistent Aspire package versions detected.
Aspire.Hosting.MauiandAspire.Hosting.DevTunnelsuse preview version13.0.0-preview.1.25560.3, whileAspire.Hosting.TestingandAspire.Hosting.AppHostuse stable version13.0.0.
For consistency and compatibility, all Aspire.Hosting packages should use the same version. Consider either:
- Using stable
13.0.0for all packages if the preview features aren't required - Using the same preview version for all packages if preview features are needed
<PackageVersion Condition=" '$(aspire)' == 'true' OR '$(aspire)' == '' " Include="Aspire.Hosting.Maui" Version="13.0.0-preview.1.25560.3" />
<PackageVersion Condition=" '$(aspire)' == 'true' OR '$(aspire)' == '' " Include="Aspire.Hosting.Testing" Version="13.0.0" />
<PackageVersion Condition=" '$(aspire)' == 'true' OR '$(aspire)' == '' " Include="CommunityToolkit.Aspire.Hosting.MailPit" Version="9.9.0" />
<PackageVersion Condition=" '$(aspire)' == 'true' OR '$(aspire)' == '' " Include="Aspire.Hosting.DevTunnels" Version="13.0.0-preview.1.25560.3" />
<PackageVersion Condition=" '$(aspire)' == 'true' OR '$(aspire)' == '' " Include="Aspire.Hosting.AppHost" Version="13.0.0" />
...te/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/Extensions/IOpenTelemetryExtensions.cs
Outdated
Show resolved
Hide resolved
...rplate/src/Client/Boilerplate.Client.Windows/Services/WindowsTelemetryEnrichmentProcessor.cs
Outdated
Show resolved
Hide resolved
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.AppHost/Program.cs
Show resolved
Hide resolved
...te/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/Extensions/IOpenTelemetryExtensions.cs
Outdated
Show resolved
Hide resolved
...te/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/Extensions/IOpenTelemetryExtensions.cs
Outdated
Show resolved
Hide resolved
...te/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/Extensions/IOpenTelemetryExtensions.cs
Outdated
Show resolved
Hide resolved
…te.Client.Maui/Extensions/IOpenTelemetryExtensions.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Yaser Moradi <ysmoradi@outlook.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.AppHost/Program.cs (1)
124-129: Windows Blazor Hybrid client not wired to server or OTLP dev tunnel
clientwindowsis only marked with.WithExplicitStart()and has neither a reference toserverWebProjectnor.WithOtlpDevTunnel(). That means:
- No explicit dependency/link to the server (unlike the MAUI devices).
- No OpenTelemetry dev tunnel wiring, which undermines the PR goal for the Windows client.
Align it with the MAUI/mobile pattern and the earlier suggestion:
- if (OperatingSystem.IsWindows()) - { - // Blazor Hybrid Windows project. - builder.AddProject("clientwindows", "../../Client/Boilerplate.Client.Windows/Boilerplate.Client.Windows.csproj") - .WithExplicitStart(); - } + if (OperatingSystem.IsWindows()) + { + // Blazor Hybrid Windows project. + builder.AddProject("clientwindows", "../../Client/Boilerplate.Client.Windows/Boilerplate.Client.Windows.csproj") + .WithExplicitStart() + .WithOtlpDevTunnel() + .WithReference(serverWebProject, tunnel); + }
🧹 Nitpick comments (5)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppChatbot.cs (1)
69-75: Consider adding defensive null check forvariablesPrompt.The initialization logic is correct. However, at line 97-98, only
supportSystemPromptis checked to verify thatStartChatwas called. SincevariablesPromptis also initialized inStartChatand used at line 110, consider adding it to the guard condition for consistency:- if (string.IsNullOrEmpty(supportSystemPrompt)) + if (string.IsNullOrEmpty(supportSystemPrompt) || string.IsNullOrEmpty(variablesPrompt)) throw new InvalidOperationException("Chat session must be started before processing messages. Call Start method first.");src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/appsettings.json (1)
39-39: Consider environment-specific trace sampling rates.A
TracesSampleRateof 1.0 (100%) may generate excessive telemetry data in high-traffic production environments, potentially increasing costs. Consider using environment-specific configuration to apply lower sampling rates in production while maintaining full sampling in development.src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Extensions/CancellationTokenSourceExtensions.cs (1)
8-19: Implementation is correct, but return value is unused.The
TryCancel()implementation correctly handlesObjectDisposedExceptionduring cancellation. However, all call sites in the codebase ignore the boolean return value, suggesting it may not be necessary.This is acceptable as-is for defensive coding, but consider whether returning
Taskinstead ofTask<bool>would better reflect actual usage patterns.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.AppHost/Program.cs (2)
132-171: MAUI device OTLP wiring is asymmetric; confirm Windows/MacCatalyst behaviorThe MAUI iOS and Android devices/emulators are correctly wired with
.WithOtlpDevTunnel().WithReference(serverWebProject, tunnel), but:
mauiapp.AddWindowsDevice()only has.WithExplicitStart().WithReference(serverWebProject).mauiapp.AddMacCatalystDevice()only has.WithExplicitStart().WithReference(serverWebProject).If the intent is that all MAUI targets participate in OpenTelemetry via the same OTLP dev tunnel, consider aligning these with the mobile pattern:
- if (OperatingSystem.IsWindows()) - { - mauiapp.AddWindowsDevice() - .WithExplicitStart() - .WithReference(serverWebProject); - } + if (OperatingSystem.IsWindows()) + { + mauiapp.AddWindowsDevice() + .WithExplicitStart() + .WithOtlpDevTunnel() + .WithReference(serverWebProject, tunnel); + } - if (OperatingSystem.IsMacOS()) - { - mauiapp.AddMacCatalystDevice() - .WithExplicitStart() - .WithReference(serverWebProject); - } + if (OperatingSystem.IsMacOS()) + { + mauiapp.AddMacCatalystDevice() + .WithExplicitStart() + .WithOtlpDevTunnel() + .WithReference(serverWebProject, tunnel); + }If Windows/MacCatalyst are meant to talk to a local/hosted collector without the tunnel, a brief code comment explaining the difference would help future readers.
141-161: Consider consolidating duplicateOperatingSystem.IsMacOS()checksYou use
OperatingSystem.IsMacOS()twice: once forAddMacCatalystDeviceand again for iOS device/simulator. Functionally fine, but you could slightly simplify:if (OperatingSystem.IsMacOS()) { mauiapp.AddMacCatalystDevice() .WithExplicitStart() .WithReference(serverWebProject); // Windows supports iOS... (existing comment) mauiapp.AddiOSDevice() .WithExplicitStart() .WithOtlpDevTunnel() .WithReference(serverWebProject, tunnel); mauiapp.AddiOSSimulator() .WithExplicitStart() .WithOtlpDevTunnel() .WithReference(serverWebProject, tunnel); }Not required, but it reduces repetition and keeps the Mac-specific config in one place.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (19)
src/Templates/Boilerplate/Bit.Boilerplate/.docs/15- Logging, OpenTelemetry and Health Checks.md(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/AppComponentBase.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Management/RolesPage.razor.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Management/UsersPage.razor.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/Boilerplate.Client.Maui.csproj(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/MauiProgram.Services.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/Services/MauiAppInsightsTelemetryInitializer.cs(0 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Windows/Boilerplate.Client.Windows.csproj(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Windows/Program.Services.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Windows/Program.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Windows/Services/WindowsAppInsightsTelemetryInitializer.cs(0 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props(4 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppChatbot.cs(3 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppHub.Chatbot.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.AppHost/Boilerplate.Server.AppHost.csproj(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.AppHost/Program.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Extensions/CancellationTokenSourceExtensions.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/appsettings.json(1 hunks)
💤 Files with no reviewable changes (2)
- src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Windows/Services/WindowsAppInsightsTelemetryInitializer.cs
- src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/Services/MauiAppInsightsTelemetryInitializer.cs
🧰 Additional context used
🪛 LanguageTool
src/Templates/Boilerplate/Bit.Boilerplate/.docs/15- Logging, OpenTelemetry and Health Checks.md
[grammar] ~258-~258: Ensure spelling is correct
Context: ...t.Core project would collect JavaScript erros and more from Browser and Blazor Hybrid...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build and test
🔇 Additional comments (15)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppChatbot.cs (2)
30-30: LGTM: Improved field naming.The rename from
variablestovariablesPromptbetter conveys that this field holds a formatted prompt string containing variable substitutions rather than a collection of variables.
110-110: LGTM: Consistent usage.The field is correctly used as a system message in the chat completion request, consistent with the rename.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppHub.Chatbot.cs (1)
46-46: LGTM!The migration to
TryCancel()provides safer cancellation handling by gracefully handling potentialObjectDisposedExceptionrace conditions when canceling ongoing message processing.src/Templates/Boilerplate/Bit.Boilerplate/.docs/15- Logging, OpenTelemetry and Health Checks.md (1)
254-256: LGTM!Documentation correctly reflects the migration of OpenTelemetry configuration to
IOpenTelemetryExtensions.cs.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/AppComponentBase.cs (1)
258-258: LGTM!The migration to
TryCancel()in bothAbort()andDisposeAsync()properly handles potential race conditions during component lifecycle management.Also applies to: 267-267
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Management/UsersPage.razor.cs (1)
113-113: LGTM!Using
TryCancel()when switching between users prevents potential race conditions and gracefully handles cancellation of in-flight data loading operations.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Management/RolesPage.razor.cs (1)
100-100: LGTM!The migration to
TryCancel()in both role selection and disposal aligns with the broader cancellation pattern, providing safer handling of concurrent cancellation scenarios.Also applies to: 401-401
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/MainLayout.razor.cs (1)
146-146: LGTM!The use of
TryCancel()properly handles cancellation during authentication state changes and component disposal, preventing potential exceptions in concurrent scenarios.Also applies to: 199-199
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.AppHost/Boilerplate.Server.AppHost.csproj (1)
15-15: Adding Aspire.Hosting.Maui to AppHost is consistent with the Aspire/Maui hosting setup.The new package reference aligns AppHost with the MAUI hosting support configured elsewhere and with the central version in Directory.Packages.props; no issues from this change.
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Windows/Program.cs (1)
28-31: Environment variable configuration provider is wired in correctly.Chaining
AddEnvironmentVariables()afterAddClientConfigurations(...)makes environment values act as overrides for client config, which is a good default for the template.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Maui/Boilerplate.Client.Maui.csproj (2)
1-1: Encoding/BOM cleanup on the<Project>element is fine.No behavioral impact; just makes diffs cleaner.
172-175: MAUI telemetry package switch to Azure Monitor OpenTelemetry exporter looks correct.The conditional references for
Newtonsoft.Json,Sentry.Maui, andAzure.Monitor.OpenTelemetry.Exporterunder the existingappInsights/sentryflags are consistent with the new OpenTelemetry-based wiring in the services code and with the central package versions.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Windows/Program.Services.cs (1)
2-7: Windows OpenTelemetry logging configuration is coherent and matches the new dependencies.
AddOpenTelemetrywith formatted messages/scopes, conditional Azure Monitor export (when a connection string exists), and OTLP export behindOTEL_EXPORTER_OTLP_ENDPOINTis a solid setup, and ordering withConfigureLoggersandAddEventLoglooks fine.Also applies to: 61-83
src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props (1)
29-96: Central package additions/updates (env vars, Aspire.Hosting.Maui, Sentry, Azure Monitor exporter) are aligned with the code changes.The new central versions back the configuration/env-var provider, Aspire hosting for MAUI, Sentry RC bump, and Azure Monitor OpenTelemetry exporter used in the client projects. Please just ensure you run at least one template build with
sentryandappInsightsvariants enabled to confirm there are no unexpected version interactions.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Windows/Boilerplate.Client.Windows.csproj (1)
40-50: Windows client package references now correctly match the new logging and configuration pipeline.The added env-var configuration provider and OTLP exporter packages, plus the switch to
Azure.Monitor.OpenTelemetry.Exporterunder the existingappInsightscondition, line up with the updated Program and logging code.
closes #11682
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.