opentelemetry-js icon indicating copy to clipboard operation
opentelemetry-js copied to clipboard

Context leak if several TracerProvider instances are used

Open Flarna opened this issue 4 years ago • 7 comments

There are several places in our codebase where context.active() is used to fetch the current context from global ContextManager.

I think this may be problematic as this easily results in "leaking" spans from one export chain into another. Following code illustrates this:

import * as assert from 'assert';

import { NodeTracerProvider } from '@opentelemetry/node';
import { InMemorySpanExporter, SimpleSpanProcessor } from '@opentelemetry/tracing';
import { context, setSpan } from '@opentelemetry/api';

const globalExporter = new InMemorySpanExporter();
const globalTracerProvider = new NodeTracerProvider();
globalTracerProvider.addSpanProcessor(new SimpleSpanProcessor(globalExporter));
globalTracerProvider.register();
const globalTracer = globalTracerProvider.getTracer('global');

const otherExporter = new InMemorySpanExporter();
const otherTracerProvider = new NodeTracerProvider();
otherTracerProvider.addSpanProcessor(new SimpleSpanProcessor(otherExporter));
const otherTracer = otherTracerProvider.getTracer('other');

describe('context lead', () => {
  beforeEach(() => {
    otherExporter.reset();
    globalExporter.reset();
  });

  it('should create independent trace if global root span is present', () => {
    const rootSpan = globalTracer.startSpan('global');
    context.with(setSpan(context.active(), rootSpan), () => {
      // assume this is in some other code part but on same callstack
      // startSpan here uses context.active() => using global context manager and therefore rootSpan as parent
      otherTracer.startSpan('other').end();
    });
    rootSpan.end();

    assert.strictEqual(globalExporter.getFinishedSpans().length, 1);
    assert.strictEqual(otherExporter.getFinishedSpans().length, 1);

    assert.strictEqual(globalExporter.getFinishedSpans()[0].parentSpanId, undefined);
    // asserts => other exports a span with parent which will be never exported via other export chain
    assert.strictEqual(otherExporter.getFinishedSpans()[0].parentSpanId, undefined);
  });
});

There two TracerProvider instances are used resulting in two independent export chains. But actually the span exported via "other" export chain refers to the "global" span as parent. Reason is that startSpan() uses context.active() therefore the global span becomes visible there.

Only if other creates its own context manager and uses it everywhere they are independent.

Using it everywhere is actually hard as e.g. BatchSpanProcessor use global context internally to suppress their exports.

There are several places in our samples/docs/API which illustrate that using more and non global TracerProvider is possible (e.g. InstrumentationAbstract.setTracerProvider(),...) But to me it looks like this will result in unexpected behavior if this is actually done by users.

Propagator API seems to be not effected as it requires to pass in a context and global context is not used automatically.

I'm not really sure how to tackle this.

  • [x] This only affects the JavaScript OpenTelemetry library
  • [x] This may affect other libraries, but I would like to get opinions here first

Flarna avatar Feb 16 '21 08:02 Flarna