-
Notifications
You must be signed in to change notification settings - Fork 438
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
[ETW Exporter] - Add Trace flags in SpanContext #1618
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1618 +/- ##
==========================================
- Coverage 85.28% 85.22% -0.06%
==========================================
Files 156 156
Lines 4978 4978
==========================================
- Hits 4245 4242 -3
- Misses 733 736 +3
|
@ThomsonTan Can you please help review it? Thanks. |
auto traceId = parentContext.IsValid() ? parentContext.trace_id() : traceId_; | ||
|
||
// Sampling based on attributes is not supported for now, so passing empty below. | ||
std::map<std::string, int> empty_attributes = {{}}; |
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.
cosmetic: It seems this file has mixed camel case and snake case.
It would be good if you could unify at least the parts you are changing.
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.
Thanks good comment. I gave it a thought, and using snake-case now, to make it consistent with the existing ETW exporter naming style. We will eventually move this component out of the core library.
Changes
ETW exporter doesn't preserve trace flags while creating span. The trace flags contain the
IsSampled
flag based on the sampling decision. This result in incorrect sampling logic execution.Also added code to copy the tracer_id from parent span to child span.
The code is taken from the sdk implementation, and a unit test is added to validate it.
For significant contributions please make sure you have completed the following items:
CHANGELOG.md
updated for non-trivial changes