Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add contextHeaderBehavior option #900

Merged
merged 3 commits into from
Nov 2, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 31 additions & 9 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

This comment was marked as spam.

This comment was marked as spam.

* 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.
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -244,8 +266,8 @@ export const defaultConfig = {
flushDelaySeconds: 30,
ignoreUrls: ['/_ah/health'],
samplingRate: 10,
contextHeaderBehavior: 'default',
bufferSize: 1000,
onUncaughtException: 'ignore',
ignoreContextHeader: false,
serviceContext: {}
};
12 changes: 11 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
49 changes: 38 additions & 11 deletions src/trace-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -43,7 +64,7 @@ export interface StackdriverTracerConfig extends TracePolicyConfig {
interface IncomingTraceContext {
traceId?: string;
spanId?: string;
options?: number;
options: number;
}

/**
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/tracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions test/plugins/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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: [],
Expand Down
105 changes: 74 additions & 31 deletions test/test-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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.');
}
});
});
});
3 changes: 2 additions & 1 deletion test/test-plugin-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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,
Expand Down
Loading