Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

Commit

Permalink
fix: Fix SpanId generation for interoperability (#126)
Browse files Browse the repository at this point in the history
* Fix SpanId formatting for interoperability

SpanIds in opencensus-node are currently generated, sent in traces, and
propagated to other services as numeric values. This isn't consistent
with other opencensus libraries (e.g.:
[opencensus-java](https://github.com/census-instrumentation/opencensus-java/blob/master/api/src/main/java/io/opencensus/trace/SpanId.java)
serializes the span id as lowercase base16). When opencensus-node is
setup with B3 propagation, this SpanId format violates the [B3
specification](https://github.com/openzipkin/b3-propagation#spanid-1)
which mandates lowercase base16 encoding. Generally speaking,
opencensus-java only accepts spans in this format so it will fail to
correctly link to any span generated by opencensus-node. The best case
scenario is a shared trace id with a missing parent span link.

This PR removes the integer parsing, and leaves it as lowercase hex
characters. I tested locally with B3 propagation where node creates the
root span and propagates it to a Java service instrumented with
opencensus-java, and verified the data is correctly ingested and
correlated in Jaeger.

* Convert spanId into a numeric value for stackdriver

Using the hex2dec library also used by
https://github.com/googleapis/cloud-trace-nodejs/ , so assuming this
dependency is okay.

* Fix formatting
  • Loading branch information
jflorencio authored and kjin committed Sep 19, 2018
1 parent 62e3a6c commit d8462a2
Show file tree
Hide file tree
Showing 8 changed files with 1,520 additions and 972 deletions.
8 changes: 3 additions & 5 deletions packages/opencensus-core/src/internal/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@

import * as crypto from 'crypto';

// Use 6 bytes of randomness only as JS numbers are doubles not 64-bit ints.
const SPAN_ID_RANDOM_BYTES = 6;
const SPAN_ID_RANDOM_BYTES = 8;

// Use the faster crypto.randomFillSync when available (Node 7+) falling back to
// using crypto.randomBytes.
Expand All @@ -32,6 +31,5 @@ const spanRandomBuffer = randomFillSync ?
() => randomBytes(SPAN_ID_RANDOM_BYTES);

export function randomSpanId() {
// tslint:disable-next-line:ban Needed to parse hexadecimal.
return parseInt(spanRandomBuffer().toString('hex'), 16).toString();
}
return spanRandomBuffer().toString('hex');
}
3 changes: 1 addition & 2 deletions packages/opencensus-propagation-b3/src/b3-format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ export class B3Format implements Propagation {
generate(): SpanContext {
return {
traceId: uuid.v4().split('-').join(''),
// tslint:disable-next-line:ban Needed to parse hexadecimal.
spanId: parseInt(crypto.randomBytes(6).toString('hex'), 16).toString(),
spanId: crypto.randomBytes(8).toString('hex'),
options: SAMPLED_VALUE
} as SpanContext;
}
Expand Down
3 changes: 1 addition & 2 deletions packages/opencensus-propagation-jaeger/src/jaeger-format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,7 @@ export class JaegerFormat implements Propagation {
generate(): SpanContext {
return {
traceId: uuid.v4().split('-').join(''),
// tslint:disable-next-line:ban Needed to parse hexadecimal.
spanId: parseInt(crypto.randomBytes(6).toString('hex'), 16).toString(),
spanId: crypto.randomBytes(8).toString('hex'),
options: SAMPLED_VALUE
} as SpanContext;
}
Expand Down
Loading

0 comments on commit d8462a2

Please sign in to comment.