feat(service): configure opentelemetry #1342
Conversation
📝 WalkthroughWalkthroughThe pull request introduces changes to the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
src/Digdir.Domain.Dialogporten.Service/Program.cs (2)
Line range hint
38-48: Update logger configuration to use OpenTelemetryThe logger configuration still depends on ApplicationInsights, but should be updated to use OpenTelemetry based on the PR objectives.
Would you like me to provide an example of how to configure Serilog with OpenTelemetry? This would include:
- Adding appropriate OpenTelemetry packages
- Configuring Serilog with OpenTelemetry sink
- Setting up proper telemetry export
Line range hint
15-67: Consider separating telemetry configurationTo improve maintainability and separation of concerns, consider:
- Moving telemetry configuration to a separate extension class
- Using configuration patterns that allow for different telemetry providers
- Adding telemetry sampling configuration for production environments
This would make it easier to:
- Switch between different telemetry providers
- Configure different sampling rates per environment
- Test telemetry integration
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- src/Digdir.Domain.Dialogporten.Service/Digdir.Domain.Dialogporten.Service.csproj (0 hunks)
- src/Digdir.Domain.Dialogporten.Service/Program.cs (4 hunks)
💤 Files with no reviewable changes (1)
- src/Digdir.Domain.Dialogporten.Service/Digdir.Domain.Dialogporten.Service.csproj
🔇 Additional comments (2)
src/Digdir.Domain.Dialogporten.Service/Program.cs (2)
Line range hint
24-36: LGTM!The error handling is robust and the telemetryConfiguration parameter passing is consistent.
66-67: Verify OpenTelemetry configuration implementationThe
ConfigureTelemetry()extension method is called, but we need to verify its implementation.✅ Verification successful
Let me gather more context about the telemetry configuration implementation.
Based on the implementation found, I can now provide the final response:
OpenTelemetry configuration follows best practices
The
ConfigureTelemetry()implementation inAspNetUtilitiesExtensions.csdemonstrates a well-structured OpenTelemetry setup that:
- Configures service name and resource attributes
- Implements proper instrumentation for ASP.NET Core and HTTP clients
- Includes PostgreSQL tracing
- Adds runtime metrics instrumentation
- Handles both Azure Monitor (production) and Application Insights (development) appropriately
- Implements smart sampling in non-development environments
- Excludes health check endpoints from tracing
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for ConfigureTelemetry implementation ast-grep --pattern 'public static $_$ ConfigureTelemetry(this $_$ builder) { $$$ }' # Check for OpenTelemetry configuration patterns rg -A 5 'AddOpenTelemetry|ConfigureOpenTelemetry'Length of output: 1386
Script:
#!/bin/bash # Search for the complete ConfigureTelemetry implementation ast-grep --pattern 'ConfigureTelemetry' # Look for the complete file content to understand the full context rg -l "ConfigureTelemetry" | xargs catLength of output: 15301
🤖 I have created a release *beep* *boop* --- ## [1.27.0](v1.26.3...v1.27.0) (2024-10-29) ### Features * Add restrictions to Transmissions reference hierarchy ([#1310](#1310)) ([e3d53ca](e3d53ca)) * **graphql:** configure opentelemetry ([#1343](#1343)) ([e31c08b](e31c08b)) * **infrastructure:** add availability test for apim ([#1327](#1327)) ([1f9fa2b](1f9fa2b)) * **service:** configure opentelemetry ([#1342](#1342)) ([513d5e4](513d5e4)) * **utils:** configure open telemetry tracing for masstransit in aspnet package ([#1344](#1344)) ([5ec3b84](5ec3b84)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: Are Almaas <arealmaas@gmail.com>
Description
Configures opentelemetry for Service. Copy pasta from WebAPI.
Dependent on #1341
Related Issue(s)
Verification
Documentation
docs-directory, Altinnpedia or a separate linked PR in altinn-studio-docs., if applicable)Summary by CodeRabbit
New Features
Bug Fixes
Refactor