Hoist activity fields to the logging scope #11211
Conversation
- Expose SpanId, TraceId and ParentId to logging scope properties - Added tests to verify the Hierarchical ID format
Co-Authored-By: Liudmila Molkova <[email protected]>
|
Do you need to capture the fields to |
I was going to but storing the async local felt strange. I'll need to experiment with that. |
That's just a tangental bit of Activity? Hosting creates the Scope, Activity and starts it; as well as Disposing the Scope and ending the Activity, and the Activity doesn't get reused, so should be ok? |
| // Arrange | ||
|
|
||
| // Generate an id we can use for the request id header (in the correct format) | ||
| var activity = new Activity("IncomingRequest"); |
There was a problem hiding this comment.
BTW @lmolkova this why I'd like this feature https://github.com/dotnet/corefx/issues/36999. It's makes unit testing possible.
Yes, you're right, it would just be storing the activity object on 2 async locals (Activity.Current and the logging scope). It does mean it would have similar issues to holding onto the HttpContext but maybe it won't be as big a problem since not much is stored on the activity currently. |
|
🆙 📅 |
ghost
deleted the
This reverts commit f7a2d3c.
* Revert "Hoist activity fields to the logging scope (#11211)" This reverts commit f7a2d3c. * Remove tests with Activity * Remove ActivityId from HostingLogScope * Enable propogation in CreateDefaultBuilder * Clean up * s/logging/loggingBuilder * Enable Activity propogation for generichost * Replace with runtime/pull/37892
Fixes #9491
PS: I need to spend some time looking at allocations before I merge this.
cc @benaadams
cc @lmolkova @SergeyKanzhelev for review