-
Notifications
You must be signed in to change notification settings - Fork 290
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
[Instrumentation.Http] Skip tagging traces when running on .NET 9+ #2314
[Instrumentation.Http] Skip tagging traces when running on .NET 9+ #2314
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2314 +/- ##
==========================================
- Coverage 73.91% 67.96% -5.95%
==========================================
Files 267 363 +96
Lines 9615 13512 +3897
==========================================
+ Hits 7107 9184 +2077
- Misses 2508 4328 +1820
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…mothyMothra/opentelemetry-dotnet-contrib-fork into 2029_httpinstrumentation_net9
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.
PR looks pretty good, but looks like there's some build issues.
Co-authored-by: Alan West <[email protected]>
Thanks!
Sigh. Visual Studio keeps saving files with Windows line endings (CR LF), but this repo expects Unix line endings (LF). |
…mothyMothra/opentelemetry-dotnet-contrib-fork into 2029_httpinstrumentation_net9
src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs
Outdated
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.
LGTM
@@ -199,16 +203,19 @@ public void OnStopActivity(Activity activity, object? payload) | |||
|
|||
if (TryFetchResponse(payload, out var response)) | |||
{ | |||
if (currentStatusCode == ActivityStatusCode.Unset) | |||
if (!IsNet9OrGreater) |
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.
Activity status, error.type are still set above in .NET9. is that intended?
Fixes #2029
Net9 added support for Http semantic convention attributes here: dotnet/runtime#104251
Changes
Merge requirement checklist
CHANGELOG.md
files updated for non-trivial changes