-
-
Notifications
You must be signed in to change notification settings - Fork 254
Add azure monitor profiler configuration to bit Boilerplate (#11419) #11420
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
WalkthroughAdds Azure Monitor Profiler integration gated by the appInsights flag: introduces a conditional central package version, adds a conditional PackageReference in the server project, and updates OpenTelemetry setup to call AddAzureMonitorProfiler() when Application Insights is enabled. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant WB as WebApplicationBuilder
participant OTel as OpenTelemetry Setup
participant AM as Azure Monitor
participant Prof as Profiler
Dev->>WB: Configure services
WB->>OTel: AddOpenTelemetryExporters()
alt appInsights == true or empty
OTel->>AM: UseAzureMonitor(...)
AM->>Prof: AddAzureMonitorProfiler()
note over AM,Prof: Profiler enabled for telemetry/export
else appInsights == false
OTel-->>AM: Skip Azure Monitor + Profiler
note over OTel: No profiler configuration applied
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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. 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 adds Azure Monitor Profiler support to the bit Boilerplate template, enabling advanced profiling capabilities for applications using Application Insights. This enhancement allows developers to get deeper insights into application performance through profiling data.
Key changes:
- Added Azure Monitor Profiler package dependency with conditional inclusion based on appInsights template parameter
- Integrated profiler configuration into the OpenTelemetry setup
- Added the necessary using statement for the profiler namespace
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Directory.Packages.props | Added version specification for Azure.Monitor.OpenTelemetry.Profiler package |
| Boilerplate.Server.Shared.csproj | Added conditional package reference for Azure Monitor Profiler |
| WebApplicationBuilderExtensions.cs | Added using statement and integrated profiler into OpenTelemetry configuration |
...ilerplate/src/Server/Boilerplate.Server.Shared/Extensions/WebApplicationBuilderExtensions.cs
Show resolved
Hide resolved
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: 0
🧹 Nitpick comments (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Extensions/WebApplicationBuilderExtensions.cs (1)
186-186: Make the profiler opt‑in to avoid unintended cost/overhead.Enable via config (e.g.,
ApplicationInsights:Profiler:Enabled) instead of always enabling when AI is configured.Apply:
- }).AddAzureMonitorProfiler(); + }); + + // Opt-in toggle: defaults to disabled + var profilerEnabled = builder.Configuration.GetValue<bool>("ApplicationInsights:Profiler:Enabled", false); + if (profilerEnabled) + { + builder.Services + .AddOpenTelemetry() + .UseAzureMonitor(options => builder.Configuration.Bind("ApplicationInsights", options)) + .AddAzureMonitorProfiler(); + }
📜 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 (3)
src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Boilerplate.Server.Shared.csproj(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Extensions/WebApplicationBuilderExtensions.cs(2 hunks)
⏰ 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 (3)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Extensions/WebApplicationBuilderExtensions.cs (1)
5-5: LGTM on the import under the appInsights guard.Namespace aligns with the new extension call.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Boilerplate.Server.Shared.csproj (1)
22-22: Conditional reference matches template gating; looks good.Condition mirrors existing AI packages and pairs with central versioning.
Please ensure the central version entry exists (it’s at Directory.Packages.props Line 93) and restores correctly in a template with
appInsights=false(the guarded code should be omitted at emit time).src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props (1)
93-93: Profiler package is beta-only — require GA or documented justification.NuGet shows Azure.Monitor.OpenTelemetry.Profiler latest 1.0.0-beta5 (no stable); Azure.Monitor.OpenTelemetry.AspNetCore is 1.3.0 (stable). For this boilerplate either replace/remove the beta, pin to a stable alternative, or add an explicit comment in src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props (line 93) explaining why the beta is required and confirming compatibility with AspNetCore 1.3.0.
closes #11419
Summary by CodeRabbit
New Features
Chores