Add required open telemetry usages to bit Boilerplate (#11265) #11266
Conversation
|
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. 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 WalkthroughCentralized package and tool version bumps. Introduces AppActivitySource for OpenTelemetry and registers its meter/source. Adds telemetry metrics: image resize histogram, ongoing conversations UpDownCounter. Improves SignalR chatbot loop with try/finally, cancellation guards, and channel completion. Updates Aspire dashboard image tag. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AppHub
participant AI
participant Metrics
Client->>AppHub: Start conversation stream
AppHub->>Metrics: ongoing_conversations_count +1
loop Read messages
AppHub->>AI: Process message (checks cancellation)
AI-->>AppHub: Response/None
AppHub-->>Client: Streamed tokens
end
AppHub->>Client: Complete stream
AppHub->>AppHub: channel.Writer.Complete()
AppHub->>Metrics: ongoing_conversations_count -1
sequenceDiagram
participant Client
participant API
participant Storage
participant Metrics
Client->>API: UploadAttachment(file)
alt NeedsResize
API->>API: Stopwatch start
API->>Storage: Write resized image
API->>Metrics: Record histogram attachment.upload_resize_duration(ms, kind)
else No resize
API->>Storage: Write original
end
API-->>Client: 200 OK
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds required OpenTelemetry usage to the bit Boilerplate template by creating a centralized activity source and enhancing telemetry instrumentation across the application.
- Creates a new
AppActivitySourceclass to provide centralized ActivitySource and Meter instances - Integrates custom metrics and tracing into OpenTelemetry configuration
- Adds telemetry tracking for chatbot conversations and image resize operations
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Shared/Services/AppActivitySource.cs | New centralized class providing ActivitySource and Meter instances for OpenTelemetry |
| src/Server/Boilerplate.Server.Shared/Extensions/WebApplicationBuilderExtensions.cs | Registers custom meter and activity source with OpenTelemetry configuration |
| src/Server/Boilerplate.Server.Api/SignalR/AppHub.Chatbot.cs | Adds conversation count tracking and proper channel cleanup |
| src/Server/Boilerplate.Server.Api/Controllers/AttachmentController.cs | Adds histogram metric for image resize duration tracking |
| src/Server/Boilerplate.Server.AppHost/Extensions/IDistributedApplicationBuilderExtensions.cs | Updates Aspire dashboard version from 9.3 to 9.4 |
| src/Server/Boilerplate.Server.AppHost/.config/dotnet-tools.json | Updates Aspire CLI version from 9.4.0 to 9.4.1 |
| src/Directory.Packages.props | Updates multiple package versions including Aspire, Hangfire, and other dependencies |
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Services/AppActivitySource.cs
Outdated
Show resolved
Hide resolved
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Services/AppActivitySource.cs
Outdated
Show resolved
Hide resolved
...rplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/AttachmentController.cs
Outdated
Show resolved
Hide resolved
...ates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppHub.Chatbot.cs
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/AttachmentController.cs (1)
188-200: Resolve potential missing namespace for Stopwatch and enrich metric with unit/description.If there’s no global using for System.Diagnostics, this won’t compile. Also, add unit/description to the histogram for better OTel semantic clarity.
Apply this diff:
- Stopwatch stopwatch = Stopwatch.StartNew(); + var stopwatch = System.Diagnostics.Stopwatch.StartNew(); @@ - AppActivitySource.CurrentMeter.CreateHistogram<double>("attachment.upload_resize_duration") - .Record(stopwatch.Elapsed.TotalMilliseconds, new KeyValuePair<string, object?>("kind", kind.ToString())); + AppActivitySource.CurrentMeter.CreateHistogram<double>( + "attachment.upload_resize_duration", + unit: "ms", + description: "Elapsed time to resize and persist an uploaded image") + .Record( + stopwatch.Elapsed.TotalMilliseconds, + new KeyValuePair<string, object?>("attachment.kind", kind.ToString()));Optional follow-up: pre-create and reuse the histogram via AppActivitySource (see my comment in AppActivitySource.cs).
🧹 Nitpick comments (3)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/AttachmentController.cs (1)
188-200: Consider adding a span around the resize/write step for trace correlation.A short Activity span helps correlate the metric with request traces and enriches diagnostics for slow uploads.
Example (outside this hunk):
using var activity = AppActivitySource.CurrentActivity.StartActivity("attachment.resize_and_write"); activity?.SetTag("attachment.kind", kind.ToString()); // do the work... activity?.SetTag("elapsed.ms", stopwatch.Elapsed.TotalMilliseconds);src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppHub.Chatbot.cs (2)
89-91: Good addition: early cancellation guard in tool functionEarly return avoids unnecessary work and scope creation when the message is canceled.
Given this function handles PII (email address, conversation history), consider masking or redacting before logging to mitigate PII leakage risks.
174-177: Redundant cancellation passing in ReadAllAsync enumerationYou pass the cancellationToken to both ReadAllAsync(cancellationToken) and WithCancellation(cancellationToken). One is sufficient. Prefer relying on ReadAllAsync’s token.
Apply:
- await foreach (var str in channel.Reader.ReadAllAsync(cancellationToken).WithCancellation(cancellationToken)) + await foreach (var str in channel.Reader.ReadAllAsync(cancellationToken)) { yield return str; }
📜 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 (7)
src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props(5 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/AttachmentController.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppHub.Chatbot.cs(3 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.AppHost/.config/dotnet-tools.json(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.AppHost/Extensions/IDistributedApplicationBuilderExtensions.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Extensions/WebApplicationBuilderExtensions.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Services/AppActivitySource.cs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/AttachmentController.cs (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Services/AppActivitySource.cs (1)
AppActivitySource(8-13)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Extensions/WebApplicationBuilderExtensions.cs (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Services/AppActivitySource.cs (1)
AppActivitySource(8-13)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppHub.Chatbot.cs (2)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Services/DiagnosticLog/DiagnosticLoggerProvider.cs (1)
ILogger(14-20)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Services/AppActivitySource.cs (1)
AppActivitySource(8-13)
⏰ 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 (5)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.AppHost/Extensions/IDistributedApplicationBuilderExtensions.cs (1)
17-19: Confirm image tag strategy for Aspire DashboardAccording to Microsoft’s container tagging guidance, using the minor tag (
9.4) will automatically float to the latest patch release in the 9.4 line. As of now, the latest published patch is 9.4.0, and 9.4.1 is pending release:
- If you intend to always pick up the newest 9.4.x dashboard image, leave the tag as
9.4.- If you require a truly immutable reference, pin to the explicit patch (e.g.,
mcr.microsoft.com/dotnet/aspire-dashboard:9.4.0today, or switch to9.4.1once it’s available).No code changes are needed here unless you specifically want to fix to a patch version.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.AppHost/.config/dotnet-tools.json (1)
5-7: LGTM: aspire.cli bump aligns with package updates.The tool version 9.4.1 is consistent with the central package updates and should keep the CLI in sync.
src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props (1)
49-55: Version bumps look coherent across Aspire, AI, MSTest, and WebView2.The updates are consistent and align with the rest of the PR changes (Aspire 9.4.1, AI 9.8.0, MSTest 3.10.2, WebView2 1.0.3405.78). No API surface changes visible here to flag.
Also applies to: 56-57, 63-63, 76-76, 106-106, 30-30, 125-126
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Extensions/WebApplicationBuilderExtensions.cs (1)
132-151: AddSource registrations are intentional and non-redundant
Both calls register different ActivitySource names:
builder.Environment.ApplicationName(your ASP .NET Core app’s entry assembly name)AppActivitySource.CurrentActivity.Name(“Boilerplate”)Since they target separate ActivitySource instances, both registrations are required.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppHub.Chatbot.cs (1)
170-181: LGTM: try/finally with UpDownCounter maintains accurate gauge for active conversationsIncrement on entry and decrement on exit ensures correctness across normal completion and cancellations.
...ates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppHub.Chatbot.cs
Show resolved
Hide resolved
...ates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/SignalR/AppHub.Chatbot.cs
Outdated
Show resolved
Hide resolved
...ilerplate/src/Server/Boilerplate.Server.Shared/Extensions/WebApplicationBuilderExtensions.cs
Show resolved
Hide resolved
...ilerplate/src/Server/Boilerplate.Server.Shared/Extensions/WebApplicationBuilderExtensions.cs
Show resolved
Hide resolved
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Services/AppActivitySource.cs
Show resolved
Hide resolved
1 participant
closes #11265
Summary by CodeRabbit
New Features
Bug Fixes
Chores