From f5d351cc49c589b1bacc11d3eb853726c8726622 Mon Sep 17 00:00:00 2001 From: Michael Goin Date: Thu, 23 Jul 2020 12:09:10 -0700 Subject: [PATCH] feat: span processors prevent exporter span generation --- .../opentelemetry-core/src/context/context.ts | 2 +- .../src/export/BatchSpanProcessor.ts | 10 +- .../src/export/SimpleSpanProcessor.ts | 8 +- .../test/export/BatchSpanProcessor.test.ts | 30 ++++++ .../test/export/SimpleSpanProcessor.test.ts | 61 +++++++++--- .../test/export/TestStackContextManager.ts | 53 +++++++++++ .../test/export/TestTracingSpanExporter.ts | 93 +++++++++++++++++++ 7 files changed, 240 insertions(+), 17 deletions(-) create mode 100644 packages/opentelemetry-tracing/test/export/TestStackContextManager.ts create mode 100644 packages/opentelemetry-tracing/test/export/TestTracingSpanExporter.ts diff --git a/packages/opentelemetry-core/src/context/context.ts b/packages/opentelemetry-core/src/context/context.ts index 1c1fe4cdbef..671523b44f8 100644 --- a/packages/opentelemetry-core/src/context/context.ts +++ b/packages/opentelemetry-core/src/context/context.ts @@ -113,4 +113,4 @@ export function setSuppressInstrumentation(context: Context, shouldSuppress: boo export function getSuppressInstrumentation(context: Context): boolean | undefined { const value = context.getValue(SUPPRESS_INSTRUMENTATION_KEY) as boolean return value -} \ No newline at end of file +} diff --git a/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts b/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts index a81e747ac8d..1e0de2341c3 100644 --- a/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts @@ -14,7 +14,8 @@ * limitations under the License. */ -import { unrefTimer } from '@opentelemetry/core'; +import { context } from '@opentelemetry/api'; +import { unrefTimer, setSuppressInstrumentation } from '@opentelemetry/core'; import { SpanProcessor } from '../SpanProcessor'; import { BufferConfig } from '../types'; import { ReadableSpan } from './ReadableSpan'; @@ -88,7 +89,12 @@ export class BatchSpanProcessor implements SpanProcessor { setTimeout(cb, 0); return; } - this._exporter.export(this._finishedSpans, cb); + + // prevent downstream exporter calls from generating spans + context.with(setSuppressInstrumentation(context.active(), true), () => { + this._exporter.export(this._finishedSpans, cb); + }); + this._finishedSpans = []; } diff --git a/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts b/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts index 294b61777a1..fbd2d90627f 100644 --- a/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts @@ -17,6 +17,8 @@ import { SpanProcessor } from '../SpanProcessor'; import { SpanExporter } from './SpanExporter'; import { ReadableSpan } from './ReadableSpan'; +import { context } from '@opentelemetry/api'; +import { setSuppressInstrumentation } from '@opentelemetry/core' /** * An implementation of the {@link SpanProcessor} that converts the {@link Span} @@ -40,7 +42,11 @@ export class SimpleSpanProcessor implements SpanProcessor { if (this._isShutdown) { return; } - this._exporter.export([span], () => {}); + + // prevent downstream exporter calls from generating spans + context.with(setSuppressInstrumentation(context.active(), true), () => { + this._exporter.export([span], () => {}); + }) } shutdown(cb: () => void = () => {}): void { diff --git a/packages/opentelemetry-tracing/test/export/BatchSpanProcessor.test.ts b/packages/opentelemetry-tracing/test/export/BatchSpanProcessor.test.ts index 73ac0a00538..c1d6cb27c44 100644 --- a/packages/opentelemetry-tracing/test/export/BatchSpanProcessor.test.ts +++ b/packages/opentelemetry-tracing/test/export/BatchSpanProcessor.test.ts @@ -23,6 +23,9 @@ import { InMemorySpanExporter, Span, } from '../../src'; +import { context } from '@opentelemetry/api'; +import { TestTracingSpanExporter } from './TestTracingSpanExporter'; +import { TestStackContextManager } from './TestStackContextManager'; function createSampledSpan(spanName: string): Span { const tracer = new BasicTracerProvider({ @@ -227,5 +230,32 @@ describe('BatchSpanProcessor', () => { }); }); }); + + describe('flushing spans with exporter triggering instrumentation', () => { + beforeEach(() => { + const contextManager = new TestStackContextManager().enable() + context.setGlobalContextManager(contextManager); + }); + + afterEach(() => { + context.disable(); + }); + + it('should prevent instrumentation prior to export', (done) => { + const testTracingExporter = new TestTracingSpanExporter() + const processor = new BatchSpanProcessor(testTracingExporter); + + const span = createSampledSpan('test'); + processor.onStart(span); + processor.onEnd(span); + + processor.forceFlush(() => { + const exporterCreatedSpans = testTracingExporter.getExporterCreatedSpans() + assert.equal(exporterCreatedSpans.length, 0) + + done(); + }); + }); + }); }); }); diff --git a/packages/opentelemetry-tracing/test/export/SimpleSpanProcessor.test.ts b/packages/opentelemetry-tracing/test/export/SimpleSpanProcessor.test.ts index 8b67013153e..eb29c69bcab 100644 --- a/packages/opentelemetry-tracing/test/export/SimpleSpanProcessor.test.ts +++ b/packages/opentelemetry-tracing/test/export/SimpleSpanProcessor.test.ts @@ -19,9 +19,11 @@ import { Span, BasicTracerProvider, InMemorySpanExporter, - SimpleSpanProcessor, + SimpleSpanProcessor } from '../../src'; -import { SpanContext, SpanKind, TraceFlags } from '@opentelemetry/api'; +import { SpanContext, SpanKind, TraceFlags, context } from '@opentelemetry/api'; +import { TestTracingSpanExporter } from './TestTracingSpanExporter'; +import { TestStackContextManager } from './TestStackContextManager'; describe('SimpleSpanProcessor', () => { const provider = new BasicTracerProvider(); @@ -80,21 +82,54 @@ describe('SimpleSpanProcessor', () => { processor.shutdown(); assert.strictEqual(exporter.getFinishedSpans().length, 0); }); + }); - describe('force flush', () => { - it('should call an async callback when flushing is complete', done => { - const processor = new SimpleSpanProcessor(exporter); - processor.forceFlush(() => { - done(); - }); + describe('force flush', () => { + it('should call an async callback when flushing is complete', done => { + const processor = new SimpleSpanProcessor(exporter); + processor.forceFlush(() => { + done(); }); + }); - it('should call an async callback when shutdown is complete', done => { - const processor = new SimpleSpanProcessor(exporter); - processor.shutdown(() => { - done(); - }); + it('should call an async callback when shutdown is complete', done => { + const processor = new SimpleSpanProcessor(exporter); + processor.shutdown(() => { + done(); }); }); }); + + describe('onEnd', () => { + beforeEach(() => { + const contextManager = new TestStackContextManager().enable() + context.setGlobalContextManager(contextManager); + }); + + afterEach(() => { + context.disable(); + }); + + it('should prevent instrumentation prior to export', () => { + const testTracingExporter = new TestTracingSpanExporter() + const processor = new SimpleSpanProcessor(testTracingExporter); + + const spanContext: SpanContext = { + traceId: 'a3cda95b652f4a1592b449d5929fda1b', + spanId: '5e0c63257de34c92', + traceFlags: TraceFlags.SAMPLED, + }; + const span = new Span( + provider.getTracer('default'), + 'span-name', + spanContext, + SpanKind.CLIENT + ); + + processor.onEnd(span); + + const exporterCreatedSpans = testTracingExporter.getExporterCreatedSpans() + assert.equal(exporterCreatedSpans.length, 0) + }); + }); }); diff --git a/packages/opentelemetry-tracing/test/export/TestStackContextManager.ts b/packages/opentelemetry-tracing/test/export/TestStackContextManager.ts new file mode 100644 index 00000000000..e9cc59cd297 --- /dev/null +++ b/packages/opentelemetry-tracing/test/export/TestStackContextManager.ts @@ -0,0 +1,53 @@ +/* + * Copyright The OpenTelemetry 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 + * + * https://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. + */ + +import { ContextManager, Context } from '@opentelemetry/context-base'; + +/** + * A test-only ContextManager that uses an in-memory stack to keep track of + * the active context. + * + * This is not intended for advanced or asynchronous use cases. + */ +export class TestStackContextManager implements ContextManager { + private _contextStack: Context[] = []; + + active(): Context { + return this._contextStack[this._contextStack.length - 1] ?? Context.ROOT_CONTEXT; + } + + with ReturnType>(context: Context, fn: T): ReturnType { + this._contextStack.push(context); + try { + return fn(); + } + finally { + this._contextStack.pop(); + } + } + + bind(target: T, context?: Context): T { + throw new Error("Method not implemented."); + } + + enable(): this { + return this; + } + + disable(): this { + return this; + } +} diff --git a/packages/opentelemetry-tracing/test/export/TestTracingSpanExporter.ts b/packages/opentelemetry-tracing/test/export/TestTracingSpanExporter.ts new file mode 100644 index 00000000000..d9af0689f59 --- /dev/null +++ b/packages/opentelemetry-tracing/test/export/TestTracingSpanExporter.ts @@ -0,0 +1,93 @@ +/* + * Copyright The OpenTelemetry 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 + * + * https://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. + */ + +import { + BasicTracerProvider, + SpanExporter, + ReadableSpan, + Tracer, + SpanProcessor +} from '../../src'; +import { ExportResult, NoopLogger, AlwaysOnSampler } from '@opentelemetry/core'; + +/** + * A test-only span exporter that naively simulates triggering instrumentation + * (creating new spans) during export. + */ +export class TestTracingSpanExporter implements SpanExporter { + private _processedSpans: ReadableSpan[] = []; + private _exporterCreatedSpans: ReadableSpan[] = []; + private _stopped = false; + private _tracer: Tracer; + + constructor() { + const tracerProvider = new BasicTracerProvider({ + logger: new NoopLogger(), + }); + + const spanProcessor: SpanProcessor = { + forceFlush: () => {}, + onStart: () => {}, + shutdown: () => {}, + onEnd: (span) => { + this._exporterCreatedSpans.push(span); + } + } + + tracerProvider.addSpanProcessor(spanProcessor) + + this._tracer = new Tracer( + { name: 'default', version: '0.0.1' }, + { sampler: new AlwaysOnSampler() }, + tracerProvider + ); + } + + export( + spans: ReadableSpan[], + resultCallback: (result: ExportResult) => void + ): void { + if (this._stopped) { + return resultCallback(ExportResult.FAILED_NOT_RETRYABLE); + } + + // Simulates an instrumented exporter by creating a span on the tracer. + const createdSpan = this._tracer.startSpan('exporter-created-span'); + createdSpan.end() + + this._processedSpans.push(...spans); + return resultCallback(ExportResult.SUCCESS); + } + + shutdown(): void { + this._stopped = true; + this._processedSpans = []; + this._exporterCreatedSpans = []; + } + + reset() { + this._processedSpans = []; + this._exporterCreatedSpans = []; + } + + getExporterCreatedSpans(): ReadableSpan[] { + return this._exporterCreatedSpans; + } + + getProcessedSpans(): ReadableSpan[] { + return this._processedSpans; + } +}