From 756d290429b673be5f78d9d36c954a5e3689794c Mon Sep 17 00:00:00 2001 From: chradek <51000525+chradek@users.noreply.github.com> Date: Thu, 3 Oct 2019 13:41:12 -0700 Subject: [PATCH] [core-http] fixes traceparent header generation (#5376) --- .../core-http/lib/policies/tracingPolicy.ts | 10 +- .../test/policies/tracingPolicyTests.ts | 91 ++++++++++++++----- 2 files changed, 70 insertions(+), 31 deletions(-) diff --git a/sdk/core/core-http/lib/policies/tracingPolicy.ts b/sdk/core/core-http/lib/policies/tracingPolicy.ts index 5380a582ab2a..305efb68d9ec 100644 --- a/sdk/core/core-http/lib/policies/tracingPolicy.ts +++ b/sdk/core/core-http/lib/policies/tracingPolicy.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { getTracer, TraceFlags } from "@azure/core-tracing"; +import { getTracer, getTraceParentHeader } from "@azure/core-tracing"; import { RequestPolicyFactory, RequestPolicy, @@ -36,11 +36,9 @@ export class TracingPolicy extends BaseRequestPolicy { try { // set headers const spanContext = span.context(); - if (spanContext.spanId && spanContext.traceId) { - request.headers.set( - "traceparent", - `${spanContext.traceId}-${spanContext.spanId}-${spanContext.traceFlags || TraceFlags.UNSAMPLED}` - ); + const traceParentHeader = getTraceParentHeader(spanContext); + if (traceParentHeader) { + request.headers.set("traceparent", traceParentHeader); const traceState = spanContext.traceState && spanContext.traceState.serialize(); // if tracestate is set, traceparent MUST be set, so only set tracestate after traceparent if (traceState) { diff --git a/sdk/core/core-http/test/policies/tracingPolicyTests.ts b/sdk/core/core-http/test/policies/tracingPolicyTests.ts index 43c07d505052..98f9456e42dd 100644 --- a/sdk/core/core-http/test/policies/tracingPolicyTests.ts +++ b/sdk/core/core-http/test/policies/tracingPolicyTests.ts @@ -2,13 +2,30 @@ // Licensed under the MIT License. import { assert } from "chai"; -import { RequestPolicy, WebResource, HttpOperationResponse, HttpHeaders, setTracer, RequestPolicyOptions, TraceFlags, NoOpTracer, SpanOptions, SpanContext, NoOpSpan } from "../../lib/coreHttp"; +import { + RequestPolicy, + WebResource, + HttpOperationResponse, + HttpHeaders, + setTracer, + RequestPolicyOptions, + TraceFlags, + NoOpTracer, + SpanOptions, + SpanContext, + NoOpSpan +} from "../../lib/coreHttp"; import { tracingPolicy } from "../../lib/policies/tracingPolicy"; class MockSpan extends NoOpSpan { private _endCalled = false; - constructor(private traceId: string, private spanId: string, private flags: TraceFlags, private state: string) { + constructor( + private traceId: string, + private spanId: string, + private flags: TraceFlags, + private state: string + ) { super(); } @@ -27,10 +44,8 @@ class MockSpan extends NoOpSpan { spanId: this.spanId, traceFlags: this.flags, traceState: { - set(_key: string, _value: string) { - }, - unset(_key: string) { - }, + set(_key: string, _value: string) {}, + unset(_key: string) {}, get(_key: string): string | undefined { return; }, @@ -38,16 +53,20 @@ class MockSpan extends NoOpSpan { return state; } } - } + }; } } class MockTracer extends NoOpTracer { - private spans: MockSpan[] = []; private _startSpanCalled = false; - constructor(private traceId = "", private spanId = "", private flags = TraceFlags.UNSAMPLED, private state = "") { + constructor( + private traceId = "", + private spanId = "", + private flags = TraceFlags.UNSAMPLED, + private state = "" + ) { super(); } @@ -69,7 +88,8 @@ class MockTracer extends NoOpTracer { const ROOT_SPAN = new MockSpan("root", "root", TraceFlags.SAMPLED, ""); -describe("tracingPolicy", function () { +describe("tracingPolicy", function() { + const TRACE_VERSION = "00"; const mockPolicy: RequestPolicy = { sendRequest(request: WebResource): Promise { @@ -108,7 +128,12 @@ describe("tracingPolicy", function () { const span = mockTracer.getStartedSpans()[0]; assert.isTrue(span.didEnd()); - assert.equal(request.headers.get("traceparent"), `${mockTraceId}-${mockSpanId}-${TraceFlags.SAMPLED}`); + const expectedFlag = "01"; + + assert.equal( + request.headers.get("traceparent"), + `${TRACE_VERSION}-${mockTraceId}-${mockSpanId}-${expectedFlag}` + ); assert.notExists(request.headers.get("tracestate")); }); @@ -130,7 +155,12 @@ describe("tracingPolicy", function () { const span = mockTracer.getStartedSpans()[0]; assert.isTrue(span.didEnd()); - assert.equal(request.headers.get("traceparent"), `${mockTraceId}-${mockSpanId}-${TraceFlags.UNSAMPLED}`); + const expectedFlag = "00"; + + assert.equal( + request.headers.get("traceparent"), + `${TRACE_VERSION}-${mockTraceId}-${mockSpanId}-${expectedFlag}` + ); assert.notExists(request.headers.get("tracestate")); }); @@ -152,7 +182,12 @@ describe("tracingPolicy", function () { const span = mockTracer.getStartedSpans()[0]; assert.isTrue(span.didEnd()); - assert.equal(request.headers.get("traceparent"), `${mockTraceId}-${mockSpanId}-${TraceFlags.SAMPLED}`); + const expectedFlag = "01"; + + assert.equal( + request.headers.get("traceparent"), + `${TRACE_VERSION}-${mockTraceId}-${mockSpanId}-${expectedFlag}` + ); assert.equal(request.headers.get("tracestate"), mockTraceState); }); @@ -166,15 +201,18 @@ describe("tracingPolicy", function () { request.spanOptions = { parent: ROOT_SPAN }; - const policy = tracingPolicy().create({ - sendRequest(request: WebResource): Promise { - return Promise.reject({ - request: request, - status: 404, - headers: new HttpHeaders() - }); - } - }, new RequestPolicyOptions()); + const policy = tracingPolicy().create( + { + sendRequest(request: WebResource): Promise { + return Promise.reject({ + request: request, + status: 404, + headers: new HttpHeaders() + }); + } + }, + new RequestPolicyOptions() + ); try { await policy.sendRequest(request); throw new Error("Test Failure"); @@ -185,7 +223,12 @@ describe("tracingPolicy", function () { const span = mockTracer.getStartedSpans()[0]; assert.isTrue(span.didEnd()); - assert.equal(request.headers.get("traceparent"), `${mockTraceId}-${mockSpanId}-${TraceFlags.SAMPLED}`); + const expectedFlag = "01"; + + assert.equal( + request.headers.get("traceparent"), + `${TRACE_VERSION}-${mockTraceId}-${mockSpanId}-${expectedFlag}` + ); assert.equal(request.headers.get("tracestate"), mockTraceState); } }); @@ -202,6 +245,4 @@ describe("tracingPolicy", function () { assert.notExists(request.headers.get("traceparent")); assert.notExists(request.headers.get("tracestate")); }); - - });