diff --git a/README.md b/README.md index 64d002c2c..293d4d658 100644 --- a/README.md +++ b/README.md @@ -36,6 +36,7 @@ Optionally, you can pass a [configuration object](src/config.ts) to the `start() require('@google-cloud/trace-agent').start({ samplingRate: 5, // sample 5 traces per second, or at most 1 every 200 milliseconds. ignoreUrls: [ /^\/ignore-me#/ ] // ignore the "/ignore-me" endpoint. + ignoreMethods: [ 'options' ] // ignore requests with OPTIONS method (case-insensitive). }); // ... ``` diff --git a/src/config.ts b/src/config.ts index 526b1de67..b59b6e7db 100644 --- a/src/config.ts +++ b/src/config.ts @@ -134,6 +134,14 @@ export interface Config { */ ignoreUrls?: Array; + /** + * Request methods that match any string in ignoreMethods will not be traced. + * matching is *not* case-sensitive (OPTIONS == options == OptiONs) + * + * No methods are ignored by default. + */ + ignoreMethods?: string[]; + /** * An upper bound on the number of traces to gather each second. If set to 0, * sampling is disabled and all traces are recorded. Sampling rates greater @@ -265,6 +273,7 @@ export const defaultConfig = { stackTraceLimit: 10, flushDelaySeconds: 30, ignoreUrls: ['/_ah/health'], + ignoreMethods: [], samplingRate: 10, contextHeaderBehavior: 'default', bufferSize: 1000, diff --git a/src/plugin-types.ts b/src/plugin-types.ts index ecf66e565..aaeb7bf2a 100644 --- a/src/plugin-types.ts +++ b/src/plugin-types.ts @@ -103,6 +103,8 @@ export interface SpanOptions { export interface RootSpanOptions extends SpanOptions { /* A URL associated with the root span, if applicable. */ url?: string; + /* A Method associated with the root span, if applicable. */ + method?: string; /** * The serialized form of an object that contains information about an * existing trace context, if it exists. diff --git a/src/plugins/plugin-hapi.ts b/src/plugins/plugin-hapi.ts index eb67f1e87..af2343a2b 100644 --- a/src/plugins/plugin-hapi.ts +++ b/src/plugins/plugin-hapi.ts @@ -51,6 +51,7 @@ function instrument( const options: PluginTypes.RootSpanOptions = { name: req.url ? (urlParse(req.url).pathname || '') : '', url: req.url, + method: req.method, traceContext: getFirstHeader(req, api.constants.TRACE_CONTEXT_HEADER_NAME), skipFrames: 2 }; diff --git a/src/plugins/plugin-koa.ts b/src/plugins/plugin-koa.ts index 926a88b54..f49c13eeb 100644 --- a/src/plugins/plugin-koa.ts +++ b/src/plugins/plugin-koa.ts @@ -56,6 +56,7 @@ function startSpanForRequest( const options = { name: req.url ? (urlParse(req.url).pathname || '') : '', url: req.url, + method: req.method, traceContext: getFirstHeader(req, api.constants.TRACE_CONTEXT_HEADER_NAME), skipFrames: 2 }; diff --git a/src/plugins/plugin-restify.ts b/src/plugins/plugin-restify.ts index bdcc7aed3..bb02b5ff5 100644 --- a/src/plugins/plugin-restify.ts +++ b/src/plugins/plugin-restify.ts @@ -51,6 +51,7 @@ function patchRestify(restify: Restify5, api: PluginTypes.Tracer) { // as a label later. name: req.path(), url: req.url, + method: req.method, traceContext: req.header(api.constants.TRACE_CONTEXT_HEADER_NAME), skipFrames: 1 }; diff --git a/src/trace-api.ts b/src/trace-api.ts index b58eb477c..600a35d48 100644 --- a/src/trace-api.ts +++ b/src/trace-api.ts @@ -192,8 +192,11 @@ export class StackdriverTracer implements Tracer { } // Consult the trace policy. - const locallyAllowed = this.policy!.shouldTrace( - {timestamp: Date.now(), url: options.url || ''}); + const locallyAllowed = this.policy!.shouldTrace({ + timestamp: Date.now(), + url: options.url || '', + method: options.method || '' + }); const remotelyAllowed = !!( incomingTraceContext.options & Constants.TRACE_OPTIONS_TRACE_ENABLED); diff --git a/src/tracing-policy.ts b/src/tracing-policy.ts index cc8422abf..eb7d346dc 100644 --- a/src/tracing-policy.ts +++ b/src/tracing-policy.ts @@ -50,6 +50,16 @@ class URLFilter implements TracePolicyPredicate { } } +class MethodsFilter implements TracePolicyPredicate { + constructor(private readonly filterMethods: string[]) {} + + shouldTrace(method: string) { + return !this.filterMethods.some((candidate) => { + return (candidate.toLowerCase() === method.toLowerCase()); + }); + } +} + /** * Options for constructing a TracePolicy instance. */ @@ -62,6 +72,10 @@ export interface TracePolicyConfig { * A field that controls a url-based filter. */ ignoreUrls: Array; + /** + * A field that controls a method filter. + */ + ignoreMethods: string[]; } /** @@ -70,6 +84,7 @@ export interface TracePolicyConfig { export class TracePolicy { private readonly sampler: TracePolicyPredicate; private readonly urlFilter: TracePolicyPredicate; + private readonly methodsFilter: TracePolicyPredicate; /** * Constructs a new TracePolicy instance. @@ -88,6 +103,11 @@ export class TracePolicy { } else { this.urlFilter = new URLFilter(config.ignoreUrls); } + if (config.ignoreMethods.length === 0) { + this.methodsFilter = {shouldTrace: () => true}; + } else { + this.methodsFilter = new MethodsFilter(config.ignoreMethods); + } } /** @@ -95,16 +115,20 @@ export class TracePolicy { * @param options Fields that help determine whether a trace should be * created. */ - shouldTrace(options: {timestamp: number, url: string}): boolean { + shouldTrace(options: {timestamp: number, url: string, method: string}): + boolean { return this.sampler.shouldTrace(options.timestamp) && - this.urlFilter.shouldTrace(options.url); + this.urlFilter.shouldTrace(options.url) && + this.methodsFilter.shouldTrace(options.method); } static always(): TracePolicy { - return new TracePolicy({samplingRate: 0, ignoreUrls: []}); + return new TracePolicy( + {samplingRate: 0, ignoreUrls: [], ignoreMethods: []}); } static never(): TracePolicy { - return new TracePolicy({samplingRate: -1, ignoreUrls: []}); + return new TracePolicy( + {samplingRate: -1, ignoreUrls: [], ignoreMethods: []}); } } diff --git a/test/plugins/common.ts b/test/plugins/common.ts index fb8ff64c9..811e94252 100644 --- a/test/plugins/common.ts +++ b/test/plugins/common.ts @@ -70,6 +70,7 @@ shimmer.wrap(trace, 'start', function(original) { rootSpanNameOverride: (name: string) => name, samplingRate: 0, ignoreUrls: [], + ignoreMethods: [], spansPerTraceSoftLimit: Infinity, spansPerTraceHardLimit: Infinity }, new TestLogger()); diff --git a/test/test-plugin-loader.ts b/test/test-plugin-loader.ts index a301e533c..8272a6477 100644 --- a/test/test-plugin-loader.ts +++ b/test/test-plugin-loader.ts @@ -46,6 +46,7 @@ describe('Trace Plugin Loader', () => { { samplingRate: 0, ignoreUrls: [], + ignoreMethods: [], enhancedDatabaseReporting: false, contextHeaderBehavior: TraceContextHeaderBehavior.DEFAULT, rootSpanNameOverride: (name: string) => name, diff --git a/test/test-trace-api.ts b/test/test-trace-api.ts index aae53d644..68bbacc0e 100644 --- a/test/test-trace-api.ts +++ b/test/test-trace-api.ts @@ -39,6 +39,7 @@ describe('Trace Interface', () => { rootSpanNameOverride: (name: string) => name, samplingRate: 0, ignoreUrls: [], + ignoreMethods: [], spansPerTraceSoftLimit: Infinity, spansPerTraceHardLimit: Infinity }, @@ -222,6 +223,18 @@ describe('Trace Interface', () => { }); }); + it('should respect filter methods', () => { + const method = 'method'; + const traceAPI = createTraceAgent({ignoreMethods: [method]}); + traceAPI.runInRootSpan({name: 'root', method}, (rootSpan) => { + assert.strictEqual(rootSpan.type, SpanType.UNTRACED); + }); + traceAPI.runInRootSpan( + {name: 'root', method: 'alternativeMethod'}, (rootSpan) => { + assert.strictEqual(rootSpan.type, SpanType.ROOT); + }); + }); + it('should respect enhancedDatabaseReporting options field', () => { [true, false].forEach((enhancedDatabaseReporting) => { const traceAPI = createTraceAgent({ diff --git a/test/test-trace-policy.ts b/test/test-trace-policy.ts index a9e2e0a07..b46430f4f 100644 --- a/test/test-trace-policy.ts +++ b/test/test-trace-policy.ts @@ -21,16 +21,42 @@ import {TracePolicy} from '../src/tracing-policy'; describe('TracePolicy', () => { describe('URL Filtering', () => { it('should not allow filtered urls', () => { - const policy = new TracePolicy( - {samplingRate: 0, ignoreUrls: ['/_ah/health', /\/book*/]}); - assert.ok(!policy.shouldTrace({timestamp: 0, url: '/_ah/health'})); - assert.ok(!policy.shouldTrace({timestamp: 0, url: '/book/test'})); + const policy = new TracePolicy({ + samplingRate: 0, + ignoreUrls: ['/_ah/health', /\/book*/], + ignoreMethods: [] + }); + assert.ok( + !policy.shouldTrace({timestamp: 0, url: '/_ah/health', method: ''})); + assert.ok( + !policy.shouldTrace({timestamp: 0, url: '/book/test', method: ''})); }); it('should allow non-filtered urls', () => { - const policy = - new TracePolicy({samplingRate: 0, ignoreUrls: ['/_ah/health']}); - assert.ok(policy.shouldTrace({timestamp: 0, url: '/_ah/background'})); + const policy = new TracePolicy( + {samplingRate: 0, ignoreUrls: ['/_ah/health'], ignoreMethods: []}); + assert.ok(policy.shouldTrace( + {timestamp: 0, url: '/_ah/background', method: ''})); + }); + }); + + describe('Method Filtering', () => { + it('should not allow filtered methods', () => { + const policy = new TracePolicy({ + samplingRate: 0, + ignoreUrls: [], + ignoreMethods: ['method1', 'method2'] + }); + assert.ok( + !policy.shouldTrace({timestamp: 0, url: '', method: 'method1'})); + assert.ok( + !policy.shouldTrace({timestamp: 0, url: '', method: 'method2'})); + }); + + it('should allow non-filtered methods', () => { + const policy = new TracePolicy( + {samplingRate: 0, ignoreUrls: [], ignoreMethods: ['method']}); + assert.ok(policy.shouldTrace({timestamp: 0, url: '', method: 'method1'})); }); }); @@ -39,14 +65,14 @@ describe('TracePolicy', () => { const testCases = [0.1, 0.5, 1, 10, 50, 150, 200, 500, 1000]; for (const testCase of testCases) { it(`should throttle traces when samplingRate = ` + testCase, () => { - const policy = - new TracePolicy({samplingRate: testCase, ignoreUrls: []}); + const policy = new TracePolicy( + {samplingRate: testCase, ignoreUrls: [], ignoreMethods: []}); const expected = NUM_SECONDS * testCase; let actual = 0; const start = Date.now(); for (let timestamp = start; timestamp < start + 1000 * NUM_SECONDS; timestamp++) { - if (policy.shouldTrace({timestamp, url: ''})) { + if (policy.shouldTrace({timestamp, url: '', method: ''})) { actual++; } } @@ -60,11 +86,12 @@ describe('TracePolicy', () => { } it('should always sample when samplingRate = 0', () => { - const policy = new TracePolicy({samplingRate: 0, ignoreUrls: []}); + const policy = + new TracePolicy({samplingRate: 0, ignoreUrls: [], ignoreMethods: []}); let numSamples = 0; const start = Date.now(); for (let timestamp = start; timestamp < start + 1000; timestamp++) { - if (policy.shouldTrace({timestamp, url: ''})) { + if (policy.shouldTrace({timestamp, url: '', method: ''})) { numSamples++; } } @@ -72,11 +99,12 @@ describe('TracePolicy', () => { }); it('should never sample when samplingRate < 0', () => { - const policy = new TracePolicy({samplingRate: -1, ignoreUrls: []}); + const policy = new TracePolicy( + {samplingRate: -1, ignoreUrls: [], ignoreMethods: []}); let numSamples = 0; const start = Date.now(); for (let timestamp = start; timestamp < start + 1000; timestamp++) { - if (policy.shouldTrace({timestamp, url: ''})) { + if (policy.shouldTrace({timestamp, url: '', method: ''})) { numSamples++; } } diff --git a/test/test-trace-web-frameworks.ts b/test/test-trace-web-frameworks.ts index 3c512949f..69bddea20 100644 --- a/test/test-trace-web-frameworks.ts +++ b/test/test-trace-web-frameworks.ts @@ -58,7 +58,7 @@ describe('Web framework tracing', () => { before(() => { testTraceModule.setCLSForTest(); testTraceModule.setPluginLoaderForTest(); - testTraceModule.start({ignoreUrls: [/ignore-me/]}); + testTraceModule.start({ignoreUrls: [/ignore-me/], ignoreMethods: []}); axios = require('axios'); });