[core-http] fixes traceparent header generation (#5376)

Azure · Oct 3, 2019 · 756d290 · 756d290
1 parent 34ba56a
commit 756d290
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 31 deletions.
diff --git 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
@@ -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,27 +44,29 @@ 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;
         },
         serialize() {
           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<HttpOperationResponse> {
@@ -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<HttpOperationResponse> {
-        return Promise.reject({
-          request: request,
-          status: 404,
-          headers: new HttpHeaders()
-        });
-      }
-    }, new RequestPolicyOptions());
+    const policy = tracingPolicy().create(
+      {
+        sendRequest(request: WebResource): Promise<HttpOperationResponse> {
+          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"));
   });
-
-
 });