-
Notifications
You must be signed in to change notification settings - Fork 773
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
Only create a sibling activity when trace id, span id or trace state differ #5136
Only create a sibling activity when trace id, span id or trace state differ #5136
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5136 +/- ##
==========================================
- Coverage 83.38% 83.10% -0.28%
==========================================
Files 297 271 -26
Lines 12531 11958 -573
==========================================
- Hits 10449 9938 -511
+ Misses 2082 2020 -62
Flags with carried forward coverage won't be shown. Click here to find out more.
|
src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs
Outdated
Show resolved
Hide resolved
@vishweshbankwar : Something like this? |
@@ -1035,6 +1038,43 @@ public async Task DiagnosticSourceExceptionCallBackIsNotReceivedForExceptionsHan | |||
Assert.Equal(2, numberOfSubscribedEvents); | |||
} | |||
|
|||
#if NET7_0_OR_GREATER |
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.
This should be NET6_0_OR_GREATER? IHttpActivityFeature
is available in net6.0.
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.
I had to add the Microsoft.Extensions.Features
package to the test app to be able to build with net6.0.
See the project file.
Should this be made conditional in the project file?
I didn't succeed right off the bat.
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.
Try this
context.Features.Get<IHttpActivityFeature>()?.Activity;
test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs
Outdated
Show resolved
Hide resolved
src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.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.
Please changelog entry before merge.
This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or Pushing will instruct the bot to automatically remove the label. This bot runs once per day. |
Who needs to review this next? The PR was marked stale. |
Apologies. Vishwesh is travelling and will be back in Jan only. One of the other approvers will re-review this. Thanks for waiting! |
src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs
Outdated
Show resolved
Hide resolved
src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs
Show resolved
Hide resolved
src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.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.
@ngruson When you have a chance, please add a changelog entry #5136 (comment) and I'll get this PR merged.
Fixes #4466
Only create a sibling activity when trace id, span id or trace state differ in the extracted activity context.
Traceflags are excluded from the comparison.
Changes
OnStartActivity
inHttpInListener