-
Notifications
You must be signed in to change notification settings - Fork 212
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
fixing the nesting of tracing #2781
Conversation
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 believe this may cause a regression in the TraceLogger late configuration that was fixed in #2541.
Perhaps, cmd.SetContext(ctx)
may need a more selective update for the TraceLogger or other stateful context items, rather than setting the entire context.
I believe the test failure output confirms this theory.
|
Thanks for the guidance, i will look if there are ways to do this. |
Hey @nitishchauhan0022 - thank you so much for your contribution. I wanted to see if you wanted any assistance getting this PR committed, or would you rather someone else try and take it over? |
Signed-off-by: ntishchauhan0022 <[email protected]>
Signed-off-by: ntishchauhan0022 <[email protected]>
1ff0861
to
df81586
Compare
@devigned @schristoff Please take a look. |
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! Thank you for adding comments describing what is happening with the span changes.
@schristoff, what do you say?
/azp run porter-integration |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Thank you so much for this contribution!
Don't forget to make another PR and add your name to our contributors file!
🎉
This reverts commit 8fbff53.
What does this change
Fixes the improper nesting of traces which looks like caused by passing of the same context which is returned after logging the config traces, making the following traces as child traces of config trace.
What issue does it fix
Closes #2563
Notes for the reviewer
I have tested the tracing which is mentioned in the issue is properly nested now.