Skip to content

Conversation

@ysmoradi
Copy link
Member

@ysmoradi ysmoradi commented Sep 18, 2025

closes #11419

Summary by CodeRabbit

  • New Features

    • Integrated Azure Monitor Profiler with OpenTelemetry when Application Insights is enabled, providing automatic runtime profiling for deeper performance diagnostics with minimal setup.
    • Enabled by default in new projects where Application Insights is on or unspecified; disable by turning off Application Insights.
  • Chores

    • Updated telemetry dependencies to include support for the Azure Monitor Profiler, aligning with existing OpenTelemetry/Azure Monitor configuration.

@ysmoradi ysmoradi requested a review from Copilot September 18, 2025 10:54
@coderabbitai
Copy link

coderabbitai bot commented Sep 18, 2025

Walkthrough

Adds 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

Cohort / File(s) Summary of changes
Central package management
.../Bit.Boilerplate/src/Directory.Packages.props
Adds conditional central package version for Azure.Monitor.OpenTelemetry.Profiler (1.0.0-beta5) when appInsights is true or empty, placed after the Azure Monitor AspNetCore entry.
Server instrumentation + references
.../Server/Boilerplate.Server.Shared/Boilerplate.Server.Shared.csproj, .../Server/Boilerplate.Server.Shared/Extensions/WebApplicationBuilderExtensions.cs
csproj: Adds conditional PackageReference for Azure.Monitor.OpenTelemetry.Profiler under the existing appInsights condition. Code: Imports the profiler namespace and chains AddAzureMonitorProfiler() to the Azure Monitor setup within the appInsights-guarded configuration.

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
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my ears at profiler delight,
New traces hop softly into night.
With Insights on, we burrow deep,
Azure watches while metrics leap.
A gentle nudge, a nimble byte—
Logs and spans in moonlit light.
Thump-thump! Deploy, and sleep tight.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Add azure monitor profiler configuration to bit Boilerplate (#11419)" clearly states the primary change — adding Azure Monitor profiler configuration to the Boilerplate — and is concise and relevant to the changeset; the parenthetical issue number is minor noise but not misleading. The title is specific enough for teammates to understand the primary change at a glance.
Linked Issues Check ✅ Passed Linked issue #11419 requests adding the Azure Monitor profiler configuration to the Boilerplate; the PR adds a conditional central package entry, a conditional PackageReference in the server project, and the AddAzureMonitorProfiler() call in the OpenTelemetry setup, which together implement the requested coding changes. Based on the provided summaries, the changes directly satisfy the issue's coding objective.
Out of Scope Changes Check ✅ Passed All modified files and added package references are directly related to enabling the Azure Monitor profiler (Directory.Packages.props, Boilerplate.Server.Shared.csproj, and WebApplicationBuilderExtensions.cs), and no unrelated files or features appear to be changed. I found no out-of-scope changes in the provided summaries.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

Copilot AI left a 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

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 0717b25 and a278bc4.

📒 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.

@ysmoradi ysmoradi merged commit 5eae9ec into bitfoundation:develop Sep 18, 2025
6 checks passed
@ysmoradi ysmoradi deleted the 11419 branch September 18, 2025 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Azure monitor profiler configuration is missing in bit Boilerplate

1 participant