Propagation-Jaeger: Enforce strictNullChecks and noUnusedLocals (#433)

mayurkale22 · web-flow · commit 2652f201a41d · 2019-03-21T14:56:20.000-07:00
* OpenCensus-Propagation-Jaeger: Enforce strictNullChecks and noUnusedLocals

* fix review comments
diff --git a/packages/opencensus-propagation-jaeger/README.md b/packages/opencensus-propagation-jaeger/README.md
@@ -1,7 +1,7 @@
-# OpenCensus Jaeger Format Propagation for Node.js
+# OpenCensus Jaeger Format Propagation
 [![Gitter chat][gitter-image]][gitter-url]
 
-OpenCensus Jaeger Format Propagation sends a span context on the wire in an HTTP request, allowing other services to create spans with the right context.
+OpenCensus [Jaeger Format Propagation](https://www.jaegertracing.io/docs/1.10/client-libraries/#propagation-format) sends a span context on the wire in an HTTP request, allowing other services to create spans with the right context.
 
 This project is still at an early stage of development. It's subject to change.
 
diff --git a/packages/opencensus-propagation-jaeger/src/jaeger-format.ts b/packages/opencensus-propagation-jaeger/src/jaeger-format.ts
@@ -15,15 +15,21 @@
  */
 
 import {HeaderGetter, HeaderSetter, Propagation, SpanContext} from '@opencensus/core';
-
 import * as crypto from 'crypto';
 import * as uuid from 'uuid';
+import {isValidSpanId, isValidTraceId} from './validators';
+
+// TRACER_STATE_HEADER_NAME is the header key used for a span's serialized
+// context.
+export const TRACER_STATE_HEADER_NAME = 'uber-trace-id';
 
-const TRACE_ID_HEADER = 'uber-trace-id';
-const DEBUG_ID_HEADER = 'jaeger-debug-id';
+// JAEGER_DEBUG_HEADER is the name of an HTTP header or a TextMap carrier key
+// which, if found in the carrier, forces the trace to be sampled as "debug"
+// trace.
+const JAEGER_DEBUG_HEADER = 'jaeger-debug-id';
 
 const DEBUG_VALUE = 2;
-const SAMPLED_VALUE = 1;
+export const SAMPLED_VALUE = 1;
 
 /**
  * Propagates span context through Jaeger trace-id propagation.
@@ -37,39 +43,26 @@ export class JaegerFormat implements Propagation {
    * @param getter
    */
   extract(getter: HeaderGetter): SpanContext|null {
-    if (getter) {
-      let debug = 0;
-      if (getter.getHeader(DEBUG_ID_HEADER)) {
-        debug = SAMPLED_VALUE;
-      }
-
-      const spanContext = {traceId: '', spanId: '', options: debug};
-
-      let header = getter.getHeader(TRACE_ID_HEADER);
-      if (!header) {
-        return spanContext;
-      }
-      if (header instanceof Array) {
-        header = header[0];
-      }
-      const parts = header.split(':');
-      if (parts.length !== 4) {
-        return spanContext;
-      }
-
-      spanContext.traceId = parts[0];
-      spanContext.spanId = parts[1];
-
-      const jflags = Number('0x' + parts[3]);
-      const sampled = jflags & SAMPLED_VALUE;
+    const debugId = this.parseHeader(getter.getHeader(JAEGER_DEBUG_HEADER));
+    const tracerStateHeader =
+        this.parseHeader(getter.getHeader(TRACER_STATE_HEADER_NAME));
 
-      debug = (jflags & DEBUG_VALUE) || debug;
+    if (!tracerStateHeader) return null;
+    const tracerStateHeaderParts = tracerStateHeader.split(':');
+    if (tracerStateHeaderParts.length !== 4) return null;
 
-      spanContext.options = (sampled || debug) ? SAMPLED_VALUE : 0;
+    const traceId = tracerStateHeaderParts[0];
+    const spanId = tracerStateHeaderParts[1];
+    const jflags = Number(
+        '0x' +
+        (isNaN(Number(tracerStateHeaderParts[3])) ?
+             SAMPLED_VALUE :
+             Number(tracerStateHeaderParts[3])));
+    const sampled = jflags & SAMPLED_VALUE;
+    const debug = (jflags & DEBUG_VALUE) || (debugId ? SAMPLED_VALUE : 0);
+    const options = (sampled || debug) ? SAMPLED_VALUE : 0;
 
-      return spanContext;
-    }
-    return null;
+    return {traceId, spanId, options};
   }
 
   /**
@@ -78,17 +71,23 @@ export class JaegerFormat implements Propagation {
    * @param spanContext
    */
   inject(setter: HeaderSetter, spanContext: SpanContext): void {
-    if (setter) {
-      let flags = '0';
-      if (spanContext.options) {
-        flags = (spanContext.options & SAMPLED_VALUE ? SAMPLED_VALUE : 0)
-                    .toString(16);
-      }
+    if (!spanContext || !isValidTraceId(spanContext.traceId) ||
+        !isValidSpanId(spanContext.spanId)) {
+      return;
+    }
 
-      const header =
-          [spanContext.traceId, spanContext.spanId, '', flags].join(':');
-      setter.setHeader(TRACE_ID_HEADER, header);
+    let flags = '0';
+    if (spanContext.options) {
+      flags = ((spanContext.options & SAMPLED_VALUE) ? SAMPLED_VALUE : 0)
+                  .toString(16);
     }
+
+    // {parent-span-id} Deprecated, most Jaeger clients ignore on the receiving
+    // side, but still include it on the sending side.
+    const header = [
+      spanContext.traceId, spanContext.spanId, /** parent-span-id */ '', flags
+    ].join(':');
+    setter.setHeader(TRACER_STATE_HEADER_NAME, header);
   }
 
   /**
@@ -99,6 +98,14 @@ export class JaegerFormat implements Propagation {
       traceId: uuid.v4().split('-').join(''),
       spanId: crypto.randomBytes(8).toString('hex'),
       options: SAMPLED_VALUE
-    } as SpanContext;
+    };
+  }
+
+  /** Converts a headers type to a string. */
+  private parseHeader(str: string|string[]|undefined): string|undefined {
+    if (Array.isArray(str)) {
+      return str[0];
+    }
+    return str;
   }
 }
diff --git a/packages/opencensus-propagation-jaeger/src/validators.ts b/packages/opencensus-propagation-jaeger/src/validators.ts
@@ -0,0 +1,72 @@
+/**
+ * Copyright 2019, OpenCensus Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+type ValidationFn = (value: string) => boolean;
+
+/**
+ * Determines if the given hex string is truely a hex value. False if value is
+ * null.
+ * @param value
+ */
+const isHex: ValidationFn = (value: string): boolean => {
+  return typeof value === 'string' && /^[0-9A-F]*$/i.test(value);
+};
+
+/**
+ * Determines if the given hex string is all zeros. False if value is null.
+ * @param value
+ */
+const isNotAllZeros: ValidationFn = (value: string): boolean => {
+  return typeof value === 'string' && !/^[0]*$/i.test(value);
+};
+
+/**
+ * Determines if the given hex string is of the given length. False if value is
+ * null.
+ * @param value
+ */
+const isLength = (length: number): ValidationFn => {
+  return (value: string): boolean => {
+    return typeof value === 'string' && value.length === length;
+  };
+};
+
+/**
+ * Compose a set of validation functions into a single validation call.
+ */
+const compose = (...fns: ValidationFn[]): ValidationFn => {
+  return (value: string) => {
+    return fns.reduce((isValid, fn) => isValid && fn(value), true);
+  };
+};
+
+/**
+ * Determines if the given traceId is valid based on section 2.2.2.1 of the
+ * Trace Context spec.
+ */
+export const isValidTraceId = compose(isHex, isNotAllZeros, isLength(32));
+
+/**
+ * Determines if the given spanId is valid based on section 2.2.2.2 of the Trace
+ * Context spec.
+ */
+export const isValidSpanId = compose(isHex, isNotAllZeros, isLength(16));
+
+/**
+ * Determines if the given option is valid based on section 2.2.3 of the Trace
+ * Context spec.
+ */
+export const isValidOption = compose(isHex, isLength(2));
diff --git a/packages/opencensus-propagation-jaeger/test/test-jaeger-format.ts b/packages/opencensus-propagation-jaeger/test/test-jaeger-format.ts
@@ -16,42 +16,56 @@
 
 import {HeaderGetter, HeaderSetter} from '@opencensus/core';
 import * as assert from 'assert';
+import {JaegerFormat, SAMPLED_VALUE, TRACER_STATE_HEADER_NAME} from '../src/';
 
-import {JaegerFormat} from '../src/';
-
-const TRACE_ID_HEADER = 'uber-trace-id';
-
-const SAMPLED_VALUE = 0x1;
-const NOT_SAMPLED_VALUE = 0x0;
+function helperGetter(value: string|string[]|undefined) {
+  const headers: {[key: string]: string|string[]|undefined} = {};
+  headers[TRACER_STATE_HEADER_NAME] = value;
+  const getter: HeaderGetter = {
+    getHeader(name: string) {
+      return headers[name];
+    }
+  };
+  return getter;
+}
 
 const jaegerFormat = new JaegerFormat();
 
 describe('JaegerPropagation', () => {
   describe('extract()', () => {
     it('should extract context of a sampled span from headers', () => {
       const spanContext = jaegerFormat.generate();
-      // disable-next-line to disable no-any check
-      // tslint:disable-next-line
-      const headers = {} as any;
-      headers[TRACE_ID_HEADER] = `${spanContext.traceId}:${
-          spanContext.spanId}::${spanContext.options}`;
+      const getter = helperGetter(`${spanContext.traceId}:${
+          spanContext.spanId}::${spanContext.options}`);
+
+      assert.deepEqual(jaegerFormat.extract(getter), spanContext);
+    });
+
+    it('should return null when header is undefined', () => {
+      const headers: {[key: string]: string|string[]|undefined} = {};
+      headers[TRACER_STATE_HEADER_NAME] = undefined;
 
       const getter: HeaderGetter = {
         getHeader(name: string) {
           return headers[name];
         }
       };
 
+      assert.deepEqual(jaegerFormat.extract(getter), null);
+    });
+
+    it('should extract data from an array', () => {
+      const spanContext = jaegerFormat.generate();
+      const getter = helperGetter(`${spanContext.traceId}:${
+          spanContext.spanId}::${spanContext.options}`);
       assert.deepEqual(jaegerFormat.extract(getter), spanContext);
     });
   });
 
   describe('inject', () => {
     it('should inject a context of a sampled span', () => {
       const spanContext = jaegerFormat.generate();
-      // disable-next-line to disable no-any check
-      // tslint:disable-next-line
-      const headers = {} as any;
+      const headers: {[key: string]: string|string[]|undefined} = {};
       const setter: HeaderSetter = {
         setHeader(name: string, value: string) {
           headers[name] = value;
@@ -66,6 +80,28 @@ describe('JaegerPropagation', () => {
       jaegerFormat.inject(setter, spanContext);
       assert.deepEqual(jaegerFormat.extract(getter), spanContext);
     });
+
+    it('should not inject empty spancontext', () => {
+      const emptySpanContext = {
+        traceId: '',
+        spanId: '',
+        options: SAMPLED_VALUE,
+      };
+      const headers: {[key: string]: string|string[]|undefined} = {};
+      const setter: HeaderSetter = {
+        setHeader(name: string, value: string) {
+          headers[name] = value;
+        }
+      };
+      const getter: HeaderGetter = {
+        getHeader(name: string) {
+          return headers[name];
+        }
+      };
+
+      jaegerFormat.inject(setter, emptySpanContext);
+      assert.deepEqual(jaegerFormat.extract(getter), null);
+    });
   });
 
 
diff --git a/packages/opencensus-propagation-jaeger/tsconfig.json b/packages/opencensus-propagation-jaeger/tsconfig.json
@@ -6,6 +6,8 @@
     "pretty": true,
     "module": "commonjs",
     "target": "es6",
+    "strictNullChecks": true,
+    "noUnusedLocals": true
   },
   "include": [
     "src/**/*.ts",
