-
Notifications
You must be signed in to change notification settings - Fork 4
chore(logging): Rename GQL operations in OTEL #3141
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
📝 WalkthroughWalkthroughAdds a new GraphQL Activity enricher class, registers it in DI, invokes renaming during ASP.NET Core instrumentation enrichment, extends OpenTelemetry ASP.NET instrumentation API to accept configuration, and adds HotChocolate.Diagnostics to project dependencies. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Digdir.Domain.Dialogporten.GraphQL/DialogportenGqlActivityEnricher.cs (1)
16-21: Root tag value likely too high-cardinality / user-influenced; consider sanitizing + tightening the label.
activity.DisplayNamecan include user-supplied operation names (and potentially verbose selection sets), which can blow up OTEL/AppInsights cardinality and make storage/queries expensive; also consider stripping newlines/control chars before exporting as a tag.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Digdir.Domain.Dialogporten.GraphQL/DialogportenGqlActivityEnricher.cs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.cs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.cs: Use file-scoped namespaces withusingdirectives outside the namespace
Use PascalCase for classes and methods in C#
Use camelCase for variables and parameters in C#
Prefer expression bodies for single-line members in C#
Usevarwhen the type is apparent in C#
Enable nullable reference types and keep entities immutable in C#
Use OneOf for union returns when applicable in C#
All code must compile withTreatWarningsAsErrors=trueand pass .NET analyzers
Files:
src/Digdir.Domain.Dialogporten.GraphQL/DialogportenGqlActivityEnricher.cs
**/*.{cs,json,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4-space indentation with LF line endings
Files:
src/Digdir.Domain.Dialogporten.GraphQL/DialogportenGqlActivityEnricher.cs
⏰ 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). (5)
- GitHub Check: Dry run deploy apps / Deploy job reindex-dialogsearch-job to test
- GitHub Check: Dry run deploy apps / Deploy job aggregate-cost-metrics-job to test
- GitHub Check: Dry run deploy apps / Deploy job sync-resource-policy-information-job to test
- GitHub Check: Dry run deploy apps / Deploy job sync-subject-resource-mappings-job to test
- GitHub Check: build / build-and-test
🔇 Additional comments (1)
src/Digdir.Domain.Dialogporten.GraphQL/DialogportenGqlActivityEnricher.cs (1)
23-30: Verify license compatibility for the referenced “copied from” implementation.
The comment points to an external repo as the origin; please confirm its license is compatible with this project’s licensing/compliance requirements (and add proper attribution if needed).
src/Digdir.Domain.Dialogporten.GraphQL/DialogportenGqlActivityEnricher.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/Digdir.Domain.Dialogporten.GraphQL/DialogportenGqlActivityEnricher.cs (1)
39-44: Consider increasing the truncation limit for better operation visibility.The 50-character limit may be too aggressive and could cut off meaningful operation details, especially for complex queries with nested fields (e.g.,
query { user { profile { name email address phone } } }). This might reduce observability in Application Insights when diagnosing specific operations.Consider increasing the limit to 100-200 characters to better balance cardinality concerns with diagnostic value:
- if (activityName.Length > 50) + if (activityName.Length > 150) { - activityName = activityName[..50]; + activityName = activityName[..150]; }Optional enhancement: For more comprehensive sanitization, you could also strip other control characters (tabs, etc.):
activityName = new string(activityName .Where(c => !char.IsControl(c) || c == ' ') .ToArray()) .Trim();However, the current approach is reasonable for simple sanitization.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Digdir.Domain.Dialogporten.GraphQL/DialogportenGqlActivityEnricher.cs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.cs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.cs: Use file-scoped namespaces withusingdirectives outside the namespace
Use PascalCase for classes and methods in C#
Use camelCase for variables and parameters in C#
Prefer expression bodies for single-line members in C#
Usevarwhen the type is apparent in C#
Enable nullable reference types and keep entities immutable in C#
Use OneOf for union returns when applicable in C#
All code must compile withTreatWarningsAsErrors=trueand pass .NET analyzers
Files:
src/Digdir.Domain.Dialogporten.GraphQL/DialogportenGqlActivityEnricher.cs
**/*.{cs,json,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4-space indentation with LF line endings
Files:
src/Digdir.Domain.Dialogporten.GraphQL/DialogportenGqlActivityEnricher.cs
🧠 Learnings (2)
📓 Common learnings
Learnt from: elsand
Repo: Altinn/dialogporten PR: 3141
File: src/Digdir.Domain.Dialogporten.GraphQL/DialogportenGqlActivityEnricher.cs:31-41
Timestamp: 2025-12-14T16:32:38.850Z
Learning: In Dialogporten GraphQL telemetry implementation (DialogportenGqlActivityEnricher), the RenameOperationName method is called from the ASP.NET Core HTTP response enrichment callback and receives the root ASP.NET Core activity directly. The GQL child activity is not accessible at this point. The custom tag "gql.activity" is set on the root activity in EnrichExecuteRequest and read from the same root activity in RenameOperationName, so no GetRootActivity() traversal is needed in RenameOperationName.
📚 Learning: 2025-12-14T16:32:38.850Z
Learnt from: elsand
Repo: Altinn/dialogporten PR: 3141
File: src/Digdir.Domain.Dialogporten.GraphQL/DialogportenGqlActivityEnricher.cs:31-41
Timestamp: 2025-12-14T16:32:38.850Z
Learning: In Dialogporten GraphQL telemetry implementation (DialogportenGqlActivityEnricher), the RenameOperationName method is called from the ASP.NET Core HTTP response enrichment callback and receives the root ASP.NET Core activity directly. The GQL child activity is not accessible at this point. The custom tag "gql.activity" is set on the root activity in EnrichExecuteRequest and read from the same root activity in RenameOperationName, so no GetRootActivity() traversal is needed in RenameOperationName.
Applied to files:
src/Digdir.Domain.Dialogporten.GraphQL/DialogportenGqlActivityEnricher.cs
🔇 Additional comments (3)
src/Digdir.Domain.Dialogporten.GraphQL/DialogportenGqlActivityEnricher.cs (3)
1-7: LGTM! Clean namespace and imports.File-scoped namespace and using directives follow the coding guidelines correctly.
9-21: LGTM! Solid implementation of the enricher.The primary constructor syntax is clean, and the
EnrichExecuteRequestoverride correctly captures the GQL activity name on the root activity for later use byRenameOperationName.
50-60: LGTM! Clean helper method.The root activity traversal logic is straightforward and correctly used in
EnrichExecuteRequest.
|
🤖 I have created a release *beep* *boop* --- ## [1.95.6](v1.95.5...v1.95.6) (2025-12-14) ### Miscellaneous Chores * **logging:** Add prefix to GQL displayname ([#3143](#3143)) ([7931de7](7931de7)) * **logging:** Increase max length for gql operation name ([#3145](#3145)) ([2817285](2817285)) * **logging:** Rename GQL operations in OTEL ([#3141](#3141)) ([19619c9](19619c9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).



Description
This improves logging granularity for GQL operations, changing
POST /graphql/{**slug}"into eg:
GQL: query { dialogById }GQL: subscription { dialogEvents }GQL: mutation { ... }User supplied operation names, ie
query myop { dialogById }, will also be included for now.Related Issue(s)
Verification
Documentation
docs-directory, Altinnpedia or a separate linked PR in altinn-studio-docs., if applicable)