fix: change SpanContext.traceFlags to mandatory #818
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Flarna
requested review from
bg451,
dyladan,
markwolff,
mayurkale22,
obecny,
OlivierAlbertini,
rochdev and
vmarchaud
as code owners
Flarna
force-pushed
the
traceflag-nonoptional
branch
from
9e981d3 to
0d174bf
Compare
Codecov Report
@@ Coverage Diff @@
## master #818 +/- ##
==========================================
- Coverage 94.14% 92.68% -1.46%
==========================================
Files 253 244 -9
Lines 11000 10774 -226
Branches 1046 1025 -21
==========================================
- Hits 10356 9986 -370
- Misses 644 788 +144
|
According to spec SpanContext represents the W3C tracestate which includes traceId, spanId and traceFlags. As a side effect a new LinkContext types was added as links don't have traceFlags according to spec.
Flarna
force-pushed
the
traceflag-nonoptional
branch
from
0d174bf to
baf5c58
Compare
obecny
reviewed
Feb 28, 2020
packages/opentelemetry-api/test/noop-implementations/noop-tracer.test.ts
Outdated
Show resolved
Hide resolved
packages/opentelemetry-core/src/context/propagation/BinaryTraceContext.ts
Outdated
Show resolved
Hide resolved
packages/opentelemetry-core/test/context/BinaryTraceContext.test.ts
Outdated
Show resolved
Hide resolved
|
One point is maybe worth to consider: |
I actually think if anything it would be the opposite. Link context is defined in terms of span context and not the other way around. I think it might be more useful to define it as |
dyladan
approved these changes
Mar 2, 2020
obecny
reviewed
Mar 5, 2020
|
Seems CI failed once again unrelated in I noticed also following warning in log which may indicate that something should be adapted in http plugin: |
|
I'm on my phone right now so I can't do it but I think this is ready to merge whenever |
OlivierAlbertini
approved these changes
Mar 6, 2020
OlivierAlbertini
added
enhancement
dyladan
added a commit
to dyladan/opentelemetry-js
that referenced
this pull request
Sep 9, 2022
* fix: change SpanContext.traceFlags to mandatory According to spec SpanContext represents the W3C tracestate which includes traceId, spanId and traceFlags. As a side effect a new LinkContext types was added as links don't have traceFlags according to spec. * chore: review findings, rename TraceFlags.UNSAMPLED to NONE * fix: build * fix: tests * fix: correct merge Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com>
pichlermarc
pushed a commit
to dynatrace-oss-contrib/opentelemetry-js
that referenced
this pull request
Dec 15, 2023
* fix: change SpanContext.traceFlags to mandatory According to spec SpanContext represents the W3C tracestate which includes traceId, spanId and traceFlags. As a side effect a new LinkContext types was added as links don't have traceFlags according to spec. * chore: review findings, rename TraceFlags.UNSAMPLED to NONE * fix: build * fix: tests * fix: correct merge Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com>
pichlermarc
pushed a commit
to dynatrace-oss-contrib/opentelemetry-js
that referenced
this pull request
Dec 15, 2023
…pen-telemetry#818) * fix hasOwnProperty check * Revert "fix hasOwnProperty check" This reverts commit 671021cb6e9732ef14ef50f69feabaa3d49d61a4. * fix(opentelemetry/instrumentation-redis) hasOwnProperty check fixes duplicate call check in my project this code was called a second time based on importing. However in the case it was called again the check would fail. By switching to hasOwnProperty it fixes this issue. * adding tests and changing stream check to not crash * fix(opentelemetry/instrumentation-redis) changing has stream check This reverts commit 5a04acc4b9ce47713c0fc5eec18cdd5ad52ab70d. * style: fix lint Co-authored-by: Jonathan Campos <jonathan@ridealto.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which problem is this PR solving?
According to spec SpanContext represents the W3C tracestate which includes traceId, spanId and traceFlags.
As a side effect a new LinkContext types was added as links don't have traceFlags according to spec.
Short description of the changes
Adapt type definitons to match spec.