VCST-4717: Update App Insights module, use System.Diagnostics.Activity instead of App Insights directly#70
VCST-4717: Update App Insights module, use System.Diagnostics.Activity instead of App Insights directly#70ksavosteev wants to merge 5 commits into
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e28b11e. Configure here.
|
|
||
| using var operation = _telemetryClient.StartOperation(requestTelemetry); | ||
| activity.DisplayName = $"POST graphql/{request.OperationName}"; | ||
| activity.SetTag("url.path", $"graphql/{request.OperationName}"); |
There was a problem hiding this comment.
Missing leading slash in url.path tag value
Medium Severity
The url.path tag is set to graphql/{OperationName} without a leading /. The OpenTelemetry semantic convention defines url.path as the RFC 3986 URI path component, which for HTTP requests always starts with /. This non-standard value will appear incorrectly in Application Insights dashboards and may break alerts or queries that expect paths starting with /. The value likely needs to be $"/graphql/{request.OperationName}".
Reviewed by Cursor Bugbot for commit e28b11e. Configure here.
|
1 participant
Description
References
QA-test:
Jira-link:
https://virtocommerce.atlassian.net/browse/VCST-4717
Artifact URL:
https://vc3prerelease.blob.core.windows.net/packages/VirtoCommerce.Xapi_3.1012.0-pr-70-c845.zip
Note
Medium Risk
Observability behavior and export paths change for all GraphQL traffic; mis-tagged activities could be dropped or under-reported in monitoring until validated in staging.
Overview
GraphQL request telemetry no longer uses
TelemetryClientor customRequestTelemetryoperations.GraphQLHttpMiddlewareWithLogsenriches the ASP.NET CoreActivity.Currentwith operation name,graphql.type, and error status/exceptions so any configured OpenTelemetry exporter (including Application Insights) receives the same signal indirectly.The plain
POST /graphqlnoise filter is reimplemented as an OpenTelemetryBaseProcessor<Activity>that clearsRecordedon/graphqlactivities missing thegraphql.typetag (i.e. not enriched by the middleware). Module startup registers it viaAddOpenTelemetry().WithTracing(...)instead ofAddApplicationInsightsTelemetryProcessor.Package/manifest alignment: drops
Microsoft.ApplicationInsights.AspNetCorefrom Xapi.Core, addsVirtoCommerce.ApplicationInsights.Core, and bumps platform/Application Insights module versions to the VCST-4717 alpha line.Reviewed by Cursor Bugbot for commit c845be8. Bugbot is set up for automated code reviews on this repo. Configure here.