Pass W3C Trace Context test suite at strictness 1 (#1406)

Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com>
open-telemetry · Aug 13, 2020 · b368d32 · b368d32
1 parent 799fdfb
commit b368d32
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 11 deletions.
diff --git a/packages/opentelemetry-core/src/context/propagation/HttpTraceContext.ts b/packages/opentelemetry-core/src/context/propagation/HttpTraceContext.ts
@@ -27,8 +27,14 @@ import { getParentSpanContext, setExtractedSpanContext } from '../context';
 
 export const TRACE_PARENT_HEADER = 'traceparent';
 export const TRACE_STATE_HEADER = 'tracestate';
-const VALID_TRACE_PARENT_REGEX = /^(?!ff)[\da-f]{2}-([\da-f]{32})-([\da-f]{16})-([\da-f]{2})(-|$)/;
+
 const VERSION = '00';
+const VERSION_PART_COUNT = 4; // Version 00 only allows the specific 4 fields.
+
+const VERSION_REGEX = /^(?!ff)[\da-f]{2}$/;
+const TRACE_ID_REGEX = /^(?![0]{32})[\da-f]{32}$/;
+const PARENT_ID_REGEX = /^(?![0]{16})[\da-f]{16}$/;
+const FLAGS_REGEX = /^[\da-f]{2}$/;
 
 /**
  * Parses information from the [traceparent] span tag and converts it into {@link SpanContext}
@@ -41,19 +47,33 @@ const VERSION = '00';
  *     For more information see {@link https://www.w3.org/TR/trace-context/}
  */
 export function parseTraceParent(traceParent: string): SpanContext | null {
-  const match = traceParent.match(VALID_TRACE_PARENT_REGEX);
+  const trimmed = traceParent.trim();
+  const traceParentParts = trimmed.split('-');
+
+  // Current version must be structured correctly.
+  // For future versions, we can grab just the parts we do support.
   if (
-    !match ||
-    match[1] === '00000000000000000000000000000000' ||
-    match[2] === '0000000000000000'
+    traceParentParts[0] === VERSION &&
+    traceParentParts.length !== VERSION_PART_COUNT
   ) {
     return null;
   }
 
+  const [version, traceId, parentId, flags] = traceParentParts;
+  const isValidParent =
+    VERSION_REGEX.test(version) &&
+    TRACE_ID_REGEX.test(traceId) &&
+    PARENT_ID_REGEX.test(parentId) &&
+    FLAGS_REGEX.test(flags);
+
+  if (!isValidParent) {
+    return null;
+  }
+
   return {
-    traceId: match[1],
-    spanId: match[2],
-    traceFlags: parseInt(match[3], 16),
+    traceId: traceId,
+    spanId: parentId,
+    traceFlags: parseInt(flags, 16),
   };
 }
 

diff --git a/packages/opentelemetry-core/src/trace/TraceState.ts b/packages/opentelemetry-core/src/trace/TraceState.ts
@@ -68,10 +68,11 @@ export class TraceState implements api.TraceState {
       .split(LIST_MEMBERS_SEPARATOR)
       .reverse() // Store in reverse so new keys (.set(...)) will be placed at the beginning
       .reduce((agg: Map<string, string>, part: string) => {
-        const i = part.indexOf(LIST_MEMBER_KEY_VALUE_SPLITTER);
+        const listMember = part.trim(); // Optional Whitespace (OWS) handling
+        const i = listMember.indexOf(LIST_MEMBER_KEY_VALUE_SPLITTER);
         if (i !== -1) {
-          const key = part.slice(0, i);
-          const value = part.slice(i + 1, part.length);
+          const key = listMember.slice(0, i);
+          const value = listMember.slice(i + 1, part.length);
           if (validateKey(key) && validateValue(value)) {
             agg.set(key, value);
           } else {

diff --git a/packages/opentelemetry-core/test/context/HttpTraceContext.test.ts b/packages/opentelemetry-core/test/context/HttpTraceContext.test.ts
@@ -147,6 +147,19 @@ describe('HttpTraceContext', () => {
       );
     });
 
+    it('should return null if matching version but extra fields (invalid)', () => {
+      // Version 00 (our current) consists of {version}-{traceId}-{parentId}-{flags}
+      carrier[TRACE_PARENT_HEADER] =
+        '00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01-extra';
+
+      assert.deepStrictEqual(
+        getExtractedSpanContext(
+          httpTraceContext.extract(Context.ROOT_CONTEXT, carrier, defaultGetter)
+        ),
+        undefined
+      );
+    });
+
     it('extracts traceparent from list of header', () => {
       carrier[TRACE_PARENT_HEADER] = [
         '00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01',
@@ -245,6 +258,19 @@ describe('HttpTraceContext', () => {
       });
     });
 
+    it('should handle OWS in tracestate list members', () => {
+      carrier[TRACE_PARENT_HEADER] =
+        '00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01';
+      carrier[TRACE_STATE_HEADER] = 'foo=1 \t , \t bar=2, \t baz=3 ';
+      const extractedSpanContext = getExtractedSpanContext(
+        httpTraceContext.extract(Context.ROOT_CONTEXT, carrier, defaultGetter)
+      );
+
+      assert.deepStrictEqual(extractedSpanContext!.traceState!.get('foo'), '1');
+      assert.deepStrictEqual(extractedSpanContext!.traceState!.get('bar'), '2');
+      assert.deepStrictEqual(extractedSpanContext!.traceState!.get('baz'), '3');
+    });
+
     it('should fail gracefully on bad responses from getter', () => {
       const ctx1 = httpTraceContext.extract(
         Context.ROOT_CONTEXT,