feat: add contextHeaderBehavior option (#900)

src/config.ts

@@ -22,7 +22,12 @@ const pluginDirectory =
 export type CLSMechanism =
     'async-hooks'|'async-listener'|'auto'|'none'|'singular';
 
-/** Available configuration options. */
+export type ContextHeaderBehavior = 'default'|'ignore'|'require';
+
+/**
+ * Available configuration options. All fields are optional. See the
+ * defaultConfig object defined in this file for default assigned values.
+ */
 export interface Config {
   /**
    * Log levels: 0=disabled, 1=error, 2=warn, 3=info, 4=debug
@@ -137,6 +142,26 @@ export interface Config {
    */
   samplingRate?: number;
 
+  /**
+   * Specifies how to use incoming trace context headers. The following options
+   * are available:
+   *   'default' -- Trace context will be propagated for incoming requests that
+   *   contain the context header. A new trace will be created for requests
+   *   without trace context headers. All traces are still subject to local
+   *   sampling and url filter policies.
+   *   'require' -- Same as default, but traces won't be created for requests
+   *   without trace context headers. This should not be set for end user-facing
+   *   services, as this header is usually set by other traced services rather
+   *   than by users.
+   *   'ignore' -- Trace context headers will always be ignored, so a new trace
+   *   with a unique ID will be created for every request. This means that a
+   *   sampling decision specified on an incoming request will be ignored.
+   *   This might be useful for aggregating traces generated by different cloud
+   *   platform projects.
+   * All traces are still subject to local tracing policy.
+   */
+  contextHeaderBehavior?: ContextHeaderBehavior;
+
   /**
    * The number of transactions we buffer before we publish to the trace
    * API, unless `flushDelaySeconds` seconds have elapsed first.
@@ -166,13 +191,10 @@ export interface Config {
   onUncaughtException?: string;
 
   /**
-   * EXPERIMENTAL:
-   * Allows to ignore the requests X-Cloud-Trace-Context header if set. Setting
-   * this to true will cause traces generated by this module to appear
-   * separately from other distributed work done by other services on behalf of
-   * the same incoming request. Setting this will also cause sampling decisions
-   * made by other distributed components to be ignored. This is useful for
-   * aggregating traces generated by different cloud platform projects.
+   * Setting this to true or false is the same as setting contextHeaderBehavior
+   * to 'ignore' or 'default' respectively. If both are explicitly set,
+   * contextHeaderBehavior will be prioritized over this value.
+   * Deprecated: This option will be removed in a future release.
    */
   ignoreContextHeader?: boolean;
 
@@ -244,8 +266,8 @@ export const defaultConfig = {
   flushDelaySeconds: 30,
   ignoreUrls: ['/_ah/health'],
   samplingRate: 10,
+  contextHeaderBehavior: 'default',
   bufferSize: 1000,
   onUncaughtException: 'ignore',
-  ignoreContextHeader: false,
   serviceContext: {}
 };

src/index.ts

@@ -60,14 +60,24 @@ function initConfig(projectConfig: Forceable<Config>):
     envSetConfig =
         require(path.resolve(process.env.GCLOUD_TRACE_CONFIG!)) as Config;
   }
+
+  // Internally, ignoreContextHeader is no longer being used, so convert the
+  // user's value into a value for contextHeaderBehavior. But let this value
+  // be overridden by the user's explicitly set value for contextHeaderBehavior.
+  const contextHeaderBehaviorUnderride = {
+    contextHeaderBehavior: projectConfig.ignoreContextHeader ? 'ignore' :
+                                                               'default'
+  };
+
   // Configuration order of precedence:
   // 1. Environment Variables
   // 2. Project Config
   // 3. Environment Variable Set Configuration File (from GCLOUD_TRACE_CONFIG)
   // 4. Default Config (as specified in './config')
   const config = extend(
       true, {[FORCE_NEW]: projectConfig[FORCE_NEW]}, defaultConfig,
-      envSetConfig, projectConfig, envConfig, {plugins: {}});
+      envSetConfig, contextHeaderBehaviorUnderride, projectConfig, envConfig,
+      {plugins: {}});
   // The empty plugins object guarantees that plugins is a plain object,
   // even if it's explicitly specified in the config to be a non-object.
 

src/trace-api.ts

@@ -28,13 +28,34 @@ import {traceWriter} from './trace-writer';
 import {TracePolicy, TracePolicyConfig} from './tracing-policy';
 import * as util from './util';
 
+/**
+ * An enumeration of the different possible types of behavior when dealing with
+ * incoming trace context. Requests are still subject to local tracing policy.
+ */
+export enum TraceContextHeaderBehavior {
+  /**
+   * Respect the trace context header if it exists; otherwise, trace the
+   * request as a new trace.
+   */
+  DEFAULT = 'default',
+  /**
+   * Respect the trace context header if it exists; otherwise, treat the
+   * request as unsampled and don't trace it.
+   */
+  REQUIRE = 'require',
+  /**
+   * Trace every request as a new trace, even if trace context exists.
+   */
+  IGNORE = 'ignore'
+}
+
 /**
  * An interface describing configuration fields read by the StackdriverTracer
  * object. This includes fields read by the trace policy.
  */
 export interface StackdriverTracerConfig extends TracePolicyConfig {
   enhancedDatabaseReporting: boolean;
-  ignoreContextHeader: boolean;
+  contextHeaderBehavior: TraceContextHeaderBehavior;
   rootSpanNameOverride: (path: string) => string;
   spansPerTraceSoftLimit: number;
   spansPerTraceHardLimit: number;
@@ -43,7 +64,7 @@ export interface StackdriverTracerConfig extends TracePolicyConfig {
 interface IncomingTraceContext {
   traceId?: string;
   spanId?: string;
-  options?: number;
+  options: number;
 }
 
 /**
@@ -151,20 +172,26 @@ export class StackdriverTracer implements Tracer {
     }
 
     // Attempt to read incoming trace context.
-    let incomingTraceContext: IncomingTraceContext = {};
-    if (isString(options.traceContext) && !this.config!.ignoreContextHeader) {
-      const parsedContext = util.parseContextFromHeader(options.traceContext);
-      if (parsedContext) {
-        incomingTraceContext = parsedContext;
-      }
+    const incomingTraceContext: IncomingTraceContext = {options: 1};
+    let parsedContext: util.TraceContext|null = null;
+    if (isString(options.traceContext) &&
+        this.config!.contextHeaderBehavior !==
+            TraceContextHeaderBehavior.IGNORE) {
+      parsedContext = util.parseContextFromHeader(options.traceContext);
+    }
+    if (parsedContext) {
+      Object.assign(incomingTraceContext, parsedContext);
+    } else if (
+        this.config!.contextHeaderBehavior ===
+        TraceContextHeaderBehavior.REQUIRE) {
+      incomingTraceContext.options = 0;
     }
 
     // Consult the trace policy.
     const locallyAllowed = this.policy!.shouldTrace(
         {timestamp: Date.now(), url: options.url || ''});
-    const remotelyAllowed = incomingTraceContext.options === undefined ||
-        !!(incomingTraceContext.options &
-           Constants.TRACE_OPTIONS_TRACE_ENABLED);
+    const remotelyAllowed = !!(
+        incomingTraceContext.options & Constants.TRACE_OPTIONS_TRACE_ENABLED);
 
     let rootContext: RootSpan&RootContext;
     // Don't create a root span if the trace policy disallows it.

src/tracing.ts

@@ -32,7 +32,7 @@ export interface TopLevelConfig {
 
 // PluginLoaderConfig extends TraceAgentConfig
 export type NormalizedConfig =
-    (TraceWriterConfig&PluginLoaderConfig&TopLevelConfig)|{enabled: false};
+    ((TraceWriterConfig&PluginLoaderConfig&TopLevelConfig)|{enabled: false});
 
 /**
  * A class that represents automatic tracing.

test/plugins/common.ts

@@ -17,7 +17,7 @@
 
 import '../override-gcp-metadata';
 import { cls, TraceCLS } from '../../src/cls';
-import { StackdriverTracer } from '../../src/trace-api';
+import { StackdriverTracer, TraceContextHeaderBehavior } from '../../src/trace-api';
 import { traceWriter } from '../../src/trace-writer';
 import { SpanType } from '../../src/constants';
 import { TestLogger } from '../logger';
@@ -66,7 +66,7 @@ shimmer.wrap(trace, 'start', function(original) {
     testTraceAgent = new StackdriverTracer('test');
     testTraceAgent.enable({
       enhancedDatabaseReporting: false,
-      ignoreContextHeader: false,
+      contextHeaderBehavior: TraceContextHeaderBehavior.DEFAULT,
       rootSpanNameOverride: (name: string) => name,
       samplingRate: 0,
       ignoreUrls: [],

test/test-config.ts

@@ -25,7 +25,7 @@ import { NormalizedConfig } from '../src/tracing';
 import { StackdriverTracer } from '../src/trace-api';
 import {Logger} from '../src/logger';
 
-describe('Behavior set by config for context propagation mechanism', () => {
+describe('Behavior set by config for CLS', () => {
   const useAH = semver.satisfies(process.version, '>=8');
   const autoMechanism =
       useAH ? TraceCLSMechanism.ASYNC_HOOKS : TraceCLSMechanism.ASYNC_LISTENER;
@@ -93,9 +93,25 @@ describe('Behavior set by config for context propagation mechanism', () => {
   }
 });
 
-describe('Behavior set by config for overriding root span name', () => {
+describe('Behavior set by config for Tracer', () => {
   let capturedConfig: NormalizedConfig|null;
 
+  // Convenience function to assert properties of capturedConfig that we want
+  // to be true on every test, and return an object with a conveniently
+  // sanitized type.
+  const getCapturedConfig = () => {
+    assert.ok(capturedConfig);
+    const config = capturedConfig!;
+    // If !config.enabled, then TSC does not permit access to other fields on
+    // config. So use this structure instead of assert.ok(config.enabled).
+    if (config.enabled) {
+      return config;
+    } else {
+      assert.fail('Configuration was not enabled.');
+      throw new Error(); // unreachable.
+    }
+  };
+
   class CaptureConfigTestTracing extends testTraceModule.TestTracing {
     constructor(config: NormalizedConfig, traceAgent: StackdriverTracer) {
       super(config, traceAgent);
@@ -116,40 +132,67 @@ describe('Behavior set by config for overriding root span name', () => {
     testTraceModule.setTracingForTest(testTraceModule.TestTracing);
   });
 
-  it('should convert a string to a function', () => {
-    testTraceModule.start({
-      rootSpanNameOverride: 'hello'
+
+  describe('Context header behavior', () => {
+    it('should copy over an explicitly-set value', () => {
+      testTraceModule.start({
+        contextHeaderBehavior: 'require'
+      });
+      const config = getCapturedConfig();
+      assert.strictEqual(config.contextHeaderBehavior, 'require');
+    });
+
+    it('should respect the value of ignoreContextHeader if not set', () => {
+      testTraceModule.start({
+        ignoreContextHeader: false
+      });
+      let config = getCapturedConfig();
+      assert.strictEqual(config.contextHeaderBehavior, 'default');
+      capturedConfig = null;
+      testTraceModule.start({
+        ignoreContextHeader: true
+      });
+      config = getCapturedConfig();
+      assert.strictEqual(config.contextHeaderBehavior, 'ignore');
+    });
+
+    it('should override the value of ignoreContextHeader if both set', () => {
+      testTraceModule.start({
+        ignoreContextHeader: false,
+        contextHeaderBehavior: 'require'
+      });
+      let config = getCapturedConfig();
+      assert.strictEqual(config.contextHeaderBehavior, 'require');
+      capturedConfig = null;
+      testTraceModule.start({
+        ignoreContextHeader: true,
+        contextHeaderBehavior: 'require'
+      });
+      config = getCapturedConfig();
+      assert.strictEqual(config.contextHeaderBehavior, 'require');
     });
-    assert.ok(capturedConfig!);
-    // Avoid using the ! operator multiple times.
-    const config = capturedConfig!;
-    // If !config.enabled, then TSC does not permit access to other fields on
-    // config. So use this structure instead of assert.ok(config.enabled).
-    if (config.enabled) {
-      assert.strictEqual(typeof config.rootSpanNameOverride, 'function');
-      assert.strictEqual(config.rootSpanNameOverride(''), 'hello');
-    } else {
-      assert.fail('Configuration was not enabled.');
-    }
   });
 
-  it('should convert a non-string, non-function to the identity fn', () => {
-    testTraceModule.start({
-      // We should make sure passing in unsupported values at least doesn't
-      // result in a crash.
-      // tslint:disable-next-line:no-any
-      rootSpanNameOverride: 2 as any
+  describe('Overriding root span name', () => {
+    it('should convert a string to a function', () => {
+      testTraceModule.start({
+        rootSpanNameOverride: 'hello'
+      });
+      const config = getCapturedConfig();
+      assert.strictEqual(typeof config.rootSpanNameOverride, 'function');
+      assert.strictEqual(config.rootSpanNameOverride(''), 'hello');
     });
-    assert.ok(capturedConfig!);
-    // Avoid using the ! operator multiple times.
-    const config = capturedConfig!;
-    // If !config.enabled, then TSC does not permit access to other fields on
-    // config. So use this structure instead of assert.ok(config.enabled).
-    if (config.enabled) {
+
+    it('should convert a non-string, non-function to the identity fn', () => {
+      testTraceModule.start({
+        // We should make sure passing in unsupported values at least doesn't
+        // result in a crash.
+        // tslint:disable-next-line:no-any
+        rootSpanNameOverride: 2 as any
+      });
+      const config = getCapturedConfig();
       assert.strictEqual(typeof config.rootSpanNameOverride, 'function');
       assert.strictEqual(config.rootSpanNameOverride('a'), 'a');
-    } else {
-      assert.fail('Configuration was not enabled.');
-    }
+    });
   });
 });

test/test-plugin-loader.ts

@@ -20,6 +20,7 @@ import * as hook from 'require-in-the-middle';
 import * as shimmer from 'shimmer';
 
 import {Logger} from '../src/logger';
+import {TraceContextHeaderBehavior} from '../src/trace-api';
 import {PluginLoader, PluginLoaderState, PluginWrapper} from '../src/trace-plugin-loader';
 
 import {TestLogger} from './logger';
@@ -46,7 +47,7 @@ describe('Trace Plugin Loader', () => {
               samplingRate: 0,
               ignoreUrls: [],
               enhancedDatabaseReporting: false,
-              ignoreContextHeader: false,
+              contextHeaderBehavior: TraceContextHeaderBehavior.DEFAULT,
               rootSpanNameOverride: (name: string) => name,
               projectId: '0',
               spansPerTraceSoftLimit: Infinity,