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

fix: respect sampled flag in Span Processors, fix associated tests #2396

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import { context } from '@opentelemetry/api';
import { context, TraceFlags } from '@opentelemetry/api';
import {
ExportResultCode,
getEnv,
Expand Down Expand Up @@ -77,6 +77,11 @@ export abstract class BatchSpanProcessorBase<T extends BufferConfig> implements
if (this._isShutdown) {
return;
}

if ((span.spanContext().traceFlags & TraceFlags.SAMPLED) === 0) {
return;
}

this._addToBuffer(span);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import { context } from '@opentelemetry/api';
import { context, TraceFlags } from '@opentelemetry/api';
import {
ExportResultCode,
globalErrorHandler,
Expand Down Expand Up @@ -50,6 +50,10 @@ export class SimpleSpanProcessor implements SpanProcessor {
return;
}

if ((span.spanContext().traceFlags & TraceFlags.SAMPLED) === 0) {
return;
}

// prevent downstream exporter calls from generating spans
context.with(suppressTracing(context.active()), () => {
this._exporter.export([span], result => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import * as assert from 'assert';
import * as sinon from 'sinon';
import { BasicTracerProvider, BufferConfig, InMemorySpanExporter, Span } from '../../../src';
import { context } from '@opentelemetry/api';
import { TestRecordOnlySampler } from './TestRecordOnlySampler';
import { TestTracingSpanExporter } from './TestTracingSpanExporter';
import { TestStackContextManager } from './TestStackContextManager';
import { BatchSpanProcessorBase } from '../../../src/export/BatchSpanProcessorBase';
Expand All @@ -38,6 +39,15 @@ function createSampledSpan(spanName: string): Span {
return span as Span;
}

function createUnsampledSpan(spanName: string): Span {
const tracer = new BasicTracerProvider({
sampler: new TestRecordOnlySampler(),
}).getTracer('default');
const span = tracer.startSpan(spanName);
span.end();
return span as Span;
}

class BatchSpanProcessor extends BatchSpanProcessorBase<BufferConfig> {
onInit() {}
onShutdown() {}
Expand Down Expand Up @@ -121,12 +131,14 @@ describe('BatchSpanProcessorBase', () => {

const span = createSampledSpan(`${name}_0`);

processor.onStart(span);
processor.onEnd(span);
assert.strictEqual(processor['_finishedSpans'].length, 1);

await processor.forceFlush();
assert.strictEqual(exporter.getFinishedSpans().length, 1);

processor.onStart(span);
processor.onEnd(span);
assert.strictEqual(processor['_finishedSpans'].length, 1);

Expand All @@ -135,12 +147,28 @@ describe('BatchSpanProcessorBase', () => {
assert.strictEqual(spy.args.length, 2);
assert.strictEqual(exporter.getFinishedSpans().length, 0);

processor.onStart(span);
processor.onEnd(span);
assert.strictEqual(spy.args.length, 2);
assert.strictEqual(processor['_finishedSpans'].length, 0);
assert.strictEqual(exporter.getFinishedSpans().length, 0);
});

it('should not export unsampled spans', async () => {
const processor = new BatchSpanProcessor(exporter, defaultBufferConfig);
const spy: sinon.SinonSpy = sinon.spy(exporter, 'export') as any;

const span = createUnsampledSpan(`${name}_0`);
Flarna marked this conversation as resolved.
Show resolved Hide resolved

processor.onStart(span);
processor.onEnd(span);

await processor.forceFlush();
assert.strictEqual(processor['_finishedSpans'].length, 0);
assert.strictEqual(exporter.getFinishedSpans().length, 0);
assert.strictEqual(spy.args.length, 0);
});

it('should export the sampled spans with buffer size reached', done => {
const clock = sinon.useFakeTimers();
const processor = new BatchSpanProcessor(exporter, defaultBufferConfig);
Expand All @@ -153,6 +181,7 @@ describe('BatchSpanProcessorBase', () => {
assert.strictEqual(exporter.getFinishedSpans().length, 0);
}
const span = createSampledSpan(`${name}_6`);
processor.onStart(span);
processor.onEnd(span);

setTimeout(async () => {
Expand All @@ -170,6 +199,7 @@ describe('BatchSpanProcessorBase', () => {
const processor = new BatchSpanProcessor(exporter, defaultBufferConfig);
for (let i = 0; i < defaultBufferConfig.maxExportBatchSize; i++) {
const span = createSampledSpan(`${name}_${i}`);
processor.onStart(span);
processor.onEnd(span);
assert.strictEqual(exporter.getFinishedSpans().length, 0);
}
Expand All @@ -188,6 +218,7 @@ describe('BatchSpanProcessorBase', () => {
const processor = new BatchSpanProcessor(exporter, defaultBufferConfig);
for (let i = 0; i < defaultBufferConfig.maxExportBatchSize; i++) {
const span = createSampledSpan(`${name}_${i}`);
processor.onStart(span);
processor.onEnd(span);
}
assert.strictEqual(exporter.getFinishedSpans().length, 0);
Expand Down Expand Up @@ -235,9 +266,11 @@ describe('BatchSpanProcessorBase', () => {
const totalSpans = defaultBufferConfig.maxExportBatchSize * 2;
for (let i = 0; i < totalSpans; i++) {
const span = createSampledSpan(`${name}_${i}`);
processor.onStart(span);
processor.onEnd(span);
}
const span = createSampledSpan(`${name}_last`);
processor.onStart(span);
processor.onEnd(span);
clock.tick(defaultBufferConfig.scheduledDelayMillis + 10);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,14 @@ import { TestStackContextManager } from './TestStackContextManager';
import { TestTracingSpanExporter } from './TestTracingSpanExporter';

describe('SimpleSpanProcessor', () => {
const provider = new BasicTracerProvider();
const exporter = new InMemorySpanExporter();
let provider: BasicTracerProvider;
let exporter: InMemorySpanExporter;


beforeEach(() => {
Flarna marked this conversation as resolved.
Show resolved Hide resolved
provider = new BasicTracerProvider();
exporter = new InMemorySpanExporter();
});

describe('constructor', () => {
it('should create a SimpleSpanProcessor instance', () => {
Expand Down Expand Up @@ -103,7 +109,7 @@ describe('SimpleSpanProcessor', () => {
const spanContext: SpanContext = {
traceId: 'a3cda95b652f4a1592b449d5929fda1b',
spanId: '5e0c63257de34c92',
traceFlags: TraceFlags.NONE,
traceFlags: TraceFlags.SAMPLED,
};
const span = new Span(
provider.getTracer('default'),
Expand All @@ -112,6 +118,7 @@ describe('SimpleSpanProcessor', () => {
spanContext,
SpanKind.CLIENT
);
processor.onStart(span);

sinon.stub(exporter, 'export').callsFake((_, callback) => {
setTimeout(() => {
Expand Down Expand Up @@ -189,6 +196,7 @@ describe('SimpleSpanProcessor', () => {
SpanKind.CLIENT
);

processor.onStart(span);
processor.onEnd(span);

const exporterCreatedSpans = testTracingExporter.getExporterCreatedSpans();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* 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 { Sampler, SamplingDecision, SamplingResult } from '@opentelemetry/api';

/** Sampler that always records but doesn't sample spans. */
export class TestRecordOnlySampler implements Sampler {
shouldSample(): SamplingResult {
return {
decision: SamplingDecision.RECORD,
};
}

toString(): string {
return 'TestRecordOnlySampler';
}
}