Propagates logs and reports 128-bit trace IDs #452
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
| public Span(long begin, long end, String name, long traceId, List<Long> parents, | ||
| long spanId, boolean remote, boolean exportable, String processId) { | ||
| this(begin, end, name, traceId, parents, spanId, remote, exportable, processId, | ||
| null); | ||
| } | ||
|
|
||
| /** | ||
| * @deprecated please use {@link SpanBuilder} |
There was a problem hiding this comment.
continuing to overload public ctors would become rediculous
| * }</pre> | ||
| * | ||
| * @see #traceIdString() | ||
| * @since 1.2.0 |
There was a problem hiding this comment.
I think it is worthwhile to backport this to 1.0 and 1.1, even though it would be annoying. The reason is that these versions will be around a long time, and truncate trace ids perpetually unless we do..
marcingrzejszczak
approved these changes
Nov 17, 2016
| * }</pre> | ||
| * | ||
| * @see #traceIdString() | ||
| * @since 1.2.0 |
| @@ -68,7 +68,7 @@ public void inject(Span span, SpanTextMap carrier) { | |||
| } | |||
| carrier.put(Span.SAMPLED_NAME, span.isExportable() ? | |||
| Span.SPAN_SAMPLED : Span.SPAN_NOT_SAMPLED); | |||
| carrier.put(Span.TRACE_ID_NAME, Span.idToHex(span.getTraceId())); | |||
| carrier.put(Span.TRACE_ID_NAME, span.traceIdString()); | |||
There was a problem hiding this comment.
I like the traceIdString method :)
This supports 128-bit traces via a new field traceIdHigh, which matches other zipkin implementations. In encoded form, the trace ID is simply twice as long (32 hex characters). With this change in, a 128-bit trace propagated will not be downgraded to 64-bits when sending downstream, reporting to Zipkin or adding to the logging context. This will be followed by a change to support initiating 128-bit traces.
codefromthecrypt
force-pushed
the
propagate-128
branch
from
cbca519 to
1027cf4
Compare
|
changed since thing to 1.0.11 |
Current coverage is 75.09% (diff: 95.34%)@@ master #452 diff @@
==========================================
Files 140 140
Lines 3026 3056 +30
Methods 0 0
Messages 0 0
Branches 419 420 +1
==========================================
+ Hits 2253 2295 +42
+ Misses 588 578 -10
+ Partials 185 183 -2
|
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.
This supports 128-bit traces via a new field traceIdHigh, which matches
other zipkin implementations. In encoded form, the trace ID is simply
twice as long (32 hex characters).
With this change in, a 128-bit trace propagated will not be downgraded
to 64-bits when sending downstream, reporting to Zipkin or adding to
the logging context.
This will be followed by a change to support initiating 128-bit traces.
See openzipkin/b3-propagation#6