-
Notifications
You must be signed in to change notification settings - Fork 774
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
add support for true in x-b3-sampled #1413
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1413 +/- ##
==========================================
- Coverage 81.28% 81.23% -0.05%
==========================================
Files 227 227
Lines 6112 6113 +1
==========================================
- Hits 4968 4966 -2
- Misses 1144 1147 +3
|
@chris-smith-zocdoc Can you add a line to the changelog as well? Thank you! |
src/OpenTelemetry.Api/CHANGELOG.md
Outdated
Released 2020-07-23 | ||
|
||
* Initial release | ||
# Changelog |
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.
some encoding got accidentaly changed to this file? there must have been some CI which detects this.
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.
didn't see that coming, let me 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.
check if u are using utf8. I saw that sometimes it changed from utf8 to utf8-BOM.
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 edited it in vs code and it claimed it saved as utf-8
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.
you solved :) what was the issue in the end?
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.
definitely line endings lol
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 after the CRLF line ending got removed.
@christopher-taormina-zocdoc can you merge from the latest master? Usually maintainers/approvers have permission to do this in PR, but this one doesn't seem to have it, so you have to do it. |
@cijothomas just merged in master. The commit says |
Thanks @christopher-taormina-zocdoc |
Fixes #1299
Changes
Adds a string for the possible
X-B3-Sampled
value oftrue
and checks the received header against both values. Setting the header for outgoing requests remains unchanged.For significant contributions please make sure you have completed the following items:
CHANGELOG.md
updated for non-trivial changes