-
Notifications
You must be signed in to change notification settings - Fork 283
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.AspNet] Improve enrich methods #1824
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1824 +/- ##
==========================================
+ Coverage 73.91% 79.13% +5.22%
==========================================
Files 267 115 -152
Lines 9615 2991 -6624
==========================================
- Hits 7107 2367 -4740
+ Misses 2508 624 -1884
Flags with carried forward coverage won't be shown. Click here to find out more. |
7bc5e2b
to
7ca5797
Compare
7ca5797
to
030d0f4
Compare
}; | ||
o.EnrichWithException = (activity, exception) => | ||
{ | ||
activity.SetTag("exceptionType", exception.GetType().ToString()); |
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.
would be nice to change this tag to something else, as exception.type would be already covered by the instrumentation library, following the otel semantic convention!
Not a blocker, as we need to do it (if we chose to), for other instrumentation too.
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.
Created separate iisue: #1828
Seems like a problem was introduced here: https://github.com/open-telemetry/opentelemetry-dotnet-contrib/actions/runs/9180095969/job/25244103909?pr=1827#step:6:11 Interesting the ci didn't run these tests. |
Fixes #766
Changes
Instrumentation.AspNet - Improve enrich methods. It propagates what we have in stable AspNetCore package.
For significant contributions please make sure you have completed the following items:
CHANGELOG.md
updated for non-trivial changes[ ] Design discussion issue #