Improve bit Boilerplate OpenTelemetry (#11136) #11137
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 WalkthroughThe changes update the boilerplate project template to enhance OpenTelemetry instrumentation by adding support for Entity Framework Core and Hangfire. Middleware and extension methods are refactored, removing Aspire-specific logic and replacing it with generalized service defaults and health check mappings. XML formatting and package references are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Builder as WebApplicationBuilder
participant App as WebApplication
Builder->>Builder: AddServerSharedServices()
Builder->>Builder: AddServiceDefaults()
Builder->>Builder: Configure OpenTelemetry (ASP.NET Core, HTTP, EF Core, Hangfire)
App->>App: MappAppHealthChecks()
App->>App: Map /health and /alive endpoints (development only)
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR improves OpenTelemetry configuration in the bit Boilerplate by removing Aspire-specific conditional compilation directives and enhancing telemetry instrumentation. The changes make OpenTelemetry features always available rather than conditionally compiled based on Aspire usage.
Key changes:
- Removed conditional compilation directives (
//#if (aspire == true)) around OpenTelemetry and health check functionality - Added Entity Framework Core and Hangfire instrumentation to OpenTelemetry tracing
- Enhanced HTTP client configuration with improved HTTP/2 and HTTP/3 connection handling
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Program.Middlewares.cs (Web) | Replace conditional MapAspire() call with always-available MappAppHealthChecks() |
| Program.Middlewares.cs (Api) | Replace conditional MapAspire() call with always-available MappAppHealthChecks() |
| WebApplicationExtensions.cs | Rename MapAspire method to MappAppHealthChecks and remove conditional compilation |
| WebApplicationBuilderExtensions.cs | Remove Aspire conditionals, add EF Core/Hangfire instrumentation, enhance HTTP client config |
| Boilerplate.Server.Shared.csproj | Add package references for EF Core and Hangfire OpenTelemetry instrumentation |
| Directory.Packages.props | Define versions for new OpenTelemetry instrumentation packages |
Comments suppressed due to low confidence (3)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/Program.Middlewares.cs:112
- The method name 'MappAppHealthChecks' appears to have a typo. It should be 'MapAppHealthChecks' (missing one 'p') to follow consistent naming conventions with other Map methods.
app.MappAppHealthChecks();
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Extensions/WebApplicationExtensions.cs:11
- The method name 'MappAppHealthChecks' appears to have a typo. It should be 'MapAppHealthChecks' (missing one 'p') to follow consistent naming conventions with other Map methods.
public static WebApplication MappAppHealthChecks(this WebApplication app)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Middlewares.cs:53
- The method name 'MappAppHealthChecks' appears to have a typo. It should be 'MapAppHealthChecks' (missing one 'p') to follow consistent naming conventions with other Map methods.
app.MappAppHealthChecks();
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props (1)
2-120: Consider separating formatting changes from functional changes.The entire file has been reformatted from 4-space to 2-space indentation. While the formatting is consistent, this wholesale formatting change makes it harder to identify the actual functional changes (adding the two OpenTelemetry package versions). Consider keeping formatting changes separate from functional changes to improve code review clarity.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Extensions/WebApplicationBuilderExtensions.cs (1)
66-72: Update the outdated comment on line 67.The method rename from
AddAspireServiceDefaultstoAddServiceDefaultsis good, but the comment "Also knows as AddServiceDefaults" is now incorrect since the method ISAddServiceDefaults.- /// Also knows as AddServiceDefaults + /// Configures service defaults for the application
📜 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 (6)
src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Middlewares.cs(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(4 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Extensions/WebApplicationExtensions.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/Program.Middlewares.cs(1 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 (8)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Program.Middlewares.cs (1)
53-53: LGTM! Middleware simplification looks good.The replacement of
app.MapAspire()withapp.MappAppHealthChecks()aligns with the goal of removing Aspire-specific logic while maintaining essential health check functionality. This change simplifies the middleware pipeline and focuses on core monitoring capabilities.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Boilerplate.Server.Shared.csproj (1)
21-22: Good addition of OpenTelemetry instrumentation packages.Adding Entity Framework Core and Hangfire instrumentation extends observability coverage to key application components, which aligns well with the PR objective of improving OpenTelemetry boilerplate.
Note that both packages are in beta versions (1.12.0-beta.2 and 1.12.0-beta.1 respectively), so ensure compatibility and stability testing in your environment.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Web/Program.Middlewares.cs (1)
112-112: Consistent middleware change across server projects.The replacement of
app.MapAspire()withapp.MappAppHealthChecks()maintains consistency with the API server middleware configuration, which is good for maintainability and uniform behavior across the application.src/Templates/Boilerplate/Bit.Boilerplate/src/Directory.Packages.props (1)
32-33: Package version definitions for OpenTelemetry instrumentation.The version definitions for the new OpenTelemetry instrumentation packages are appropriate and support the Central Package Management approach. The beta versions are acceptable for these specialized instrumentation libraries.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Extensions/WebApplicationExtensions.cs (1)
11-28: Well-implemented health check extension method.The
MappAppHealthChecksmethod is a solid replacement for the removedMapAspirefunctionality. Key strengths:
- Security-conscious: Restricts health check endpoints to development environments only
- Flexible monitoring: Provides both comprehensive (
/health) and minimal (/alive) health check endpoints- Clear documentation: Includes helpful security guidance and endpoint descriptions
- Proper implementation: Follows .NET extension method conventions correctly
This focused approach is preferable to the broader Aspire service mappings for a boilerplate template.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Shared/Extensions/WebApplicationBuilderExtensions.cs (3)
25-25: LGTM! Method call correctly updated to match the renamed method.The change maintains consistency with the method rename from
AddAspireServiceDefaultstoAddServiceDefaults.
88-92: Good addition for HTTP connection performance optimization.Enabling multiple HTTP/2 and HTTP/3 connections via
SocketsHttpHandleris a beneficial performance enhancement that allows for better connection multiplexing.
119-120: Required OpenTelemetry Packages Are PresentI’ve confirmed that the shared project file (
Boilerplate.Server.Shared.csproj) includes:
<PackageReference Include="OpenTelemetry.Instrumentation.EntityFrameworkCore" />(v1.12.0-beta.2)<PackageReference Include="OpenTelemetry.Instrumentation.Hangfire" />(v1.12.0-beta.1)The calls to
.AddEntityFrameworkCoreInstrumentation()and.AddHangfireInstrumentation()inWebApplicationBuilderExtensions.csare fully supported. Merging as-is.
1 participant
closes #11136
Summary by CodeRabbit
New Features
/healthand/alive(development environment only).Refactor
Style