Skip to content

Commit

Permalink
feat: add incomingRequestSpanNameOverride option
Browse files Browse the repository at this point in the history
  • Loading branch information
kjin committed Jul 31, 2018
1 parent 9893bb3 commit 1c72ba0
Show file tree
Hide file tree
Showing 14 changed files with 40 additions and 93 deletions.
14 changes: 7 additions & 7 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,13 @@ export interface Config {
enhancedDatabaseReporting?: boolean;

/**
* If set to true, traces that represent incoming HTTP spans will prefer
* the header value under key Tracer#constants.TRACE_SPAN_NAME_OVERRIDE to
* the request path. This option should only be enabled if this server's
* endpoints aren't publicly exposed, as it allows misbehaving clients to
* set span names to arbitrary values.
* A value that can be used to override names of spans that represent
* incoming requests. If specified as a string, the string will be used
* to replace all such span names; if specified as a function, the function
* will be invoked with the request path as an argument, and its return value
* will be used as the span name.
*/
useSpanNameOverrideHeader?: boolean;
incomingRequestSpanNameOverride?: string|((path: string) => string);

/**
* The maximum number of characters reported on a label value. This value
Expand Down Expand Up @@ -202,7 +202,7 @@ export const defaultConfig = {
logLevel: 1,
enabled: true,
enhancedDatabaseReporting: false,
useSpanNameOverrideHeader: false,
incomingRequestSpanNameOverride: (path: string) => path,
maximumLabelValueSize: 512,
plugins: {
// enable all by default
Expand Down
6 changes: 0 additions & 6 deletions src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,6 @@ export const Constants = {
/** Header that carries trace context across Google infrastructure. */
TRACE_CONTEXT_HEADER_NAME: 'x-cloud-trace-context',

/**
* A header that can be used to override the span name of an incoming HTTP
* request.
*/
TRACE_SPAN_NAME_OVERRIDE: 'x-cloud-trace-span-name-override',

/** Header that is used to identify outgoing http made by the agent. */
TRACE_AGENT_REQUEST_HEADER: 'x-cloud-trace-agent-request',

Expand Down
7 changes: 7 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,13 @@ function initConfig(projectConfig: Forceable<Config>):
Constants.TRACE_SERVICE_LABEL_VALUE_LIMIT) {
config.maximumLabelValueSize = Constants.TRACE_SERVICE_LABEL_VALUE_LIMIT;
}
// Make incomingRequestSpanNameOverride a function if not already.
if (typeof config.incomingRequestSpanNameOverride === 'string') {
const spanName = config.incomingRequestSpanNameOverride;
config.incomingRequestSpanNameOverride = () => spanName;
} else if (typeof config.incomingRequestSpanNameOverride !== 'function') {
config.incomingRequestSpanNameOverride = (path: string) => path;
}

// If the CLS mechanism is set to auto-determined, decide now what it should
// be.
Expand Down
3 changes: 2 additions & 1 deletion src/plugin-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import {Config} from './config';
import {Constants, SpanType} from './constants';
import {StackdriverTracerConfig} from './trace-api';
import {TraceLabels} from './trace-labels';
import {TraceContext} from './util';

Expand Down Expand Up @@ -121,7 +122,7 @@ export interface Tracer {
* Gets the current configuration, or throws if it can't be retrieved
* because the Trace Agent was not disabled.
*/
getConfig(): Config;
getConfig(): StackdriverTracerConfig;

/**
* Runs the given function in a root span corresponding to an incoming
Expand Down
14 changes: 3 additions & 11 deletions src/plugins/plugin-connect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,9 @@ function getFirstHeader(req: IncomingMessage, key: string): string|null {
}

function getSpanName(tracer: PluginTypes.Tracer, req: Request): string {
// For the span name:
// 1. Use the TRACE_SPAN_NAME_OVERRIDE header.
// 2. If non-existent, use the path name.
let name;
if (tracer.getConfig().useSpanNameOverrideHeader) {
name = getFirstHeader(req, tracer.constants.TRACE_SPAN_NAME_OVERRIDE);
}
if (!name) {
name = req.originalUrl ? (urlParse(req.originalUrl).pathname || '') : '';
}
return name;
const name =
req.originalUrl ? (urlParse(req.originalUrl).pathname || '') : '';
return tracer.getConfig().incomingRequestSpanNameOverride(name);
}

function createMiddleware(api: PluginTypes.Tracer):
Expand Down
17 changes: 1 addition & 16 deletions src/plugins/plugin-express.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,28 +30,13 @@ const methods: Array<keyof express_4.Application> =

const SUPPORTED_VERSIONS = '4.x';

function getSpanName(
tracer: PluginTypes.Tracer, req: express_4.Request): string {
// For the span name:
// 1. Use the TRACE_SPAN_NAME_OVERRIDE header.
// 2. If non-existent, use the path name.
let name;
if (tracer.getConfig().useSpanNameOverrideHeader) {
name = req.get(tracer.constants.TRACE_SPAN_NAME_OVERRIDE);
}
if (!name) {
name = req.path;
}
return name;
}

function patchModuleRoot(express: Express4Module, api: PluginTypes.Tracer) {
const labels = api.labels;
function middleware(
req: express_4.Request, res: express_4.Response,
next: express_4.NextFunction) {
const options: PluginTypes.RootSpanOptions = {
name: getSpanName(api, req),
name: api.getConfig().incomingRequestSpanNameOverride(req.path),
traceContext: req.get(api.constants.TRACE_CONTEXT_HEADER_NAME),
url: req.originalUrl,
skipFrames: 1
Expand Down
13 changes: 2 additions & 11 deletions src/plugins/plugin-hapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,8 @@ function getFirstHeader(req: IncomingMessage, key: string): string|null {
}

function getSpanName(tracer: PluginTypes.Tracer, req: IncomingMessage): string {
// For the span name:
// 1. Use the TRACE_SPAN_NAME_OVERRIDE header.
// 2. If non-existent, use the path name.
let name;
if (tracer.getConfig().useSpanNameOverrideHeader) {
name = getFirstHeader(req, tracer.constants.TRACE_SPAN_NAME_OVERRIDE);
}
if (!name) {
name = req.url ? (urlParse(req.url).pathname || '') : '';
}
return name;
const name = req.url ? (urlParse(req.url).pathname || '') : '';
return tracer.getConfig().incomingRequestSpanNameOverride(name);
}

function instrument<T>(
Expand Down
13 changes: 2 additions & 11 deletions src/plugins/plugin-koa.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,8 @@ function getFirstHeader(req: IncomingMessage, key: string): string|null {
}

function getSpanName(tracer: PluginTypes.Tracer, req: IncomingMessage): string {
// For the span name:
// 1. Use the TRACE_SPAN_NAME_OVERRIDE header.
// 2. If non-existent, use the path name.
let name;
if (tracer.getConfig().useSpanNameOverrideHeader) {
name = getFirstHeader(req, tracer.constants.TRACE_SPAN_NAME_OVERRIDE);
}
if (!name) {
name = req.url ? (urlParse(req.url).pathname || '') : '';
}
return name;
const name = req.url ? (urlParse(req.url).pathname || '') : '';
return tracer.getConfig().incomingRequestSpanNameOverride(name);
}

function startSpanForRequest<T>(
Expand Down
16 changes: 1 addition & 15 deletions src/plugins/plugin-restify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,6 @@ function unpatchRestify(restify: Restify5) {
shimmer.unwrap(restify, 'createServer');
}

function getSpanName(tracer: PluginTypes.Tracer, req: Request): string {
// For the span name:
// 1. Use the TRACE_SPAN_NAME_OVERRIDE header.
// 2. If non-existent, use the path name.
let name;
if (tracer.getConfig().useSpanNameOverrideHeader) {
name = req.header(tracer.constants.TRACE_SPAN_NAME_OVERRIDE);
}
if (!name) {
name = req.path();
}
return name;
}

function patchRestify(restify: Restify5, api: PluginTypes.Tracer) {
shimmer.wrap(restify, 'createServer', createServerWrap);

Expand All @@ -63,7 +49,7 @@ function patchRestify(restify: Restify5, api: PluginTypes.Tracer) {
const options = {
// we use the path part of the url as the span name and add the full url
// as a label later.
name: getSpanName(api, req),
name: api.getConfig().incomingRequestSpanNameOverride(req.path()),
url: req.url,
traceContext: req.header(api.constants.TRACE_CONTEXT_HEADER_NAME),
skipFrames: 1
Expand Down
4 changes: 2 additions & 2 deletions src/trace-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import * as is from 'is';
import * as uuid from 'uuid';

import {cls, RootContext} from './cls';
import {Config} from './config';
import {Constants, SpanType} from './constants';
import {Func, RootSpan, RootSpanOptions, Span, SpanOptions, Tracer} from './plugin-types';
import {RootSpanData, UNCORRELATED_CHILD_SPAN, UNCORRELATED_ROOT_SPAN, UNTRACED_CHILD_SPAN, UNTRACED_ROOT_SPAN} from './span-data';
Expand All @@ -36,6 +35,7 @@ export interface StackdriverTracerConfig extends
TracingPolicy.TracePolicyConfig {
enhancedDatabaseReporting: boolean;
ignoreContextHeader: boolean;
incomingRequestSpanNameOverride: (path: string) => string;
}

interface IncomingTraceContext {
Expand Down Expand Up @@ -127,7 +127,7 @@ export class StackdriverTracer implements Tracer {
return !!this.config && this.config.enhancedDatabaseReporting;
}

getConfig(): Config {
getConfig(): StackdriverTracerConfig {
if (!this.config) {
throw new Error('Configuration is not available.');
}
Expand Down
1 change: 1 addition & 0 deletions test/plugins/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ shimmer.wrap(trace, 'start', function(original) {
testTraceAgent.enable({
enhancedDatabaseReporting: false,
ignoreContextHeader: false,
incomingRequestSpanNameOverride: (path: string) => path,
samplingRate: 0
}, logger());
testTraceAgent.policy = new TracingPolicy.TraceAllPolicy();
Expand Down
1 change: 1 addition & 0 deletions test/test-plugin-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ describe('Trace Plugin Loader', () => {
ignoreUrls: [],
enhancedDatabaseReporting: false,
ignoreContextHeader: false,
incomingRequestSpanNameOverride: (path: string) => path,
projectId: '0'
},
config),
Expand Down
1 change: 1 addition & 0 deletions test/test-trace-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ describe('Trace Interface', () => {
{
enhancedDatabaseReporting: false,
ignoreContextHeader: false,
incomingRequestSpanNameOverride: (path: string) => path,
samplingRate: 0
},
config),
Expand Down
23 changes: 10 additions & 13 deletions test/test-trace-web-frameworks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,9 +305,7 @@ describe('Web framework tracing', () => {
{name: 'outer'}, async (span) => {
assert.ok(testTraceModule.get().isRealSpan(span));
// Hit an endpoint with a query parameter.
await axios.get(
`http://localhost:${port}/hello?this-is=dog`,
{headers: {[spanNameOverrideKey]: '/goodbye'}});
await axios.get(`http://localhost:${port}/hello?this-is=dog`);
span!.endSpan();
});
assert.strictEqual(testTraceModule.getSpans().length, 3);
Expand All @@ -332,10 +330,6 @@ describe('Web framework tracing', () => {
stackTrace.stack_frame[0].method_name, expectedTopStackFrame);
});

it('doesn\'t read span name header when assoc. option is off', () => {
assert.strictEqual(serverSpan.name, '/hello');
});

it('doesn\'t include query parameters in span name', () => {
assert.strictEqual(
serverSpan.name.indexOf('dog'), -1,
Expand All @@ -345,22 +339,25 @@ describe('Web framework tracing', () => {

it('uses the span name override header when assoc. option is on',
async () => {
testTraceModule.get().getConfig().useSpanNameOverrideHeader = true;
const oldSpanNameOverride = testTraceModule.get()
.getConfig()
.incomingRequestSpanNameOverride;
testTraceModule.get().getConfig().incomingRequestSpanNameOverride =
(path: string) => `${path}-goodbye`;
const spanNameOverrideKey =
testTraceModule.get().constants.TRACE_SPAN_NAME_OVERRIDE;
await testTraceModule.get().runInRootSpan(
{name: 'outer'}, async (span) => {
assert.ok(testTraceModule.get().isRealSpan(span));
// Specify a header which overrides the span name.
await axios.get(
`http://localhost:${port}/hello`,
{headers: {[spanNameOverrideKey]: '/goodbye'}});
await axios.get(`http://localhost:${port}/hello`);
span!.endSpan();
});
assert.strictEqual(testTraceModule.getSpans().length, 3);
const serverSpan = testTraceModule.getOneSpan(isServerSpan);
assert.strictEqual(serverSpan.name, '/goodbye');
testTraceModule.get().getConfig().useSpanNameOverrideHeader = false;
assert.strictEqual(serverSpan.name, '/hello-goodbye');
testTraceModule.get().getConfig().incomingRequestSpanNameOverride =
oldSpanNameOverride;
});
});
});
Expand Down

0 comments on commit 1c72ba0

Please sign in to comment.