Skip to content

Commit

Permalink
feat: simplify active span logic (open-telemetry#1589)
Browse files Browse the repository at this point in the history 
Co-authored-by: Bartlomiej Obecny <bobecny@gmail.com>
  • Loading branch information
@mwear @obecny
mwear and obecny authored Oct 14, 2020
1 parent be720b4 commit 1c27690
Show file tree
Hide file tree
Showing 11 changed files with 86 additions and 96 deletions.
30 changes: 8 additions & 22 deletions packages/opentelemetry-api/src/context/context.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import { Span, SpanContext } from '../';
import { NoopSpan, Span, SpanContext } from '../';
import { Context, createContextKey } from '@opentelemetry/context-base';

/**
Expand All @@ -23,9 +23,7 @@ import { Context, createContextKey } from '@opentelemetry/context-base';
const ACTIVE_SPAN_KEY = createContextKey(
'OpenTelemetry Context Key ACTIVE_SPAN'
);
const EXTRACTED_SPAN_CONTEXT_KEY = createContextKey(
'OpenTelemetry Context Key EXTRACTED_SPAN_CONTEXT'
);

/**
* Shared key for indicating if instrumentation should be suppressed beyond
* this current scope.
Expand Down Expand Up @@ -54,29 +52,17 @@ export function setActiveSpan(context: Context, span: Span): Context {
}

/**
* Get the extracted span context from a context
*
* @param context context to get span context from
*/
export function getExtractedSpanContext(
context: Context
): SpanContext | undefined {
return (
(context.getValue(EXTRACTED_SPAN_CONTEXT_KEY) as SpanContext) || undefined
);
}

/**
* Set the extracted span context on a context
* Wrap extracted span context in a NoopSpan and set as active span in a new
* context
*
* @param context context to set span context on
* @param spanContext span context to set
* @param context context to set active span on
* @param spanContext span context to be wrapped
*/
export function setExtractedSpanContext(
context: Context,
spanContext: SpanContext
): Context {
return context.setValue(EXTRACTED_SPAN_CONTEXT_KEY, spanContext);
return setActiveSpan(context, new NoopSpan(spanContext));
}

/**
Expand All @@ -89,7 +75,7 @@ export function setExtractedSpanContext(
export function getParentSpanContext(
context: Context
): SpanContext | undefined {
return getActiveSpan(context)?.context() || getExtractedSpanContext(context);
return getActiveSpan(context)?.context();
}

/**
Expand Down
4 changes: 2 additions & 2 deletions packages/opentelemetry-api/src/trace/NoopTracer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { Span, SpanOptions, Tracer, SpanContext } from '..';
import { Context } from '@opentelemetry/context-base';
import { NoopSpan, NOOP_SPAN } from './NoopSpan';
import { isSpanContextValid } from './spancontext-utils';
import { getExtractedSpanContext } from '../context/context';
import { getActiveSpan } from '../context/context';

/**
* No-op implementations of {@link Tracer}.
Expand All @@ -31,7 +31,7 @@ export class NoopTracer implements Tracer {
// startSpan starts a noop span.
startSpan(name: string, options?: SpanOptions, context?: Context): Span {
const parent = options?.parent;
const parentFromContext = context && getExtractedSpanContext(context);
const parentFromContext = context && getActiveSpan(context)?.context();
if (isSpanContext(parent) && isSpanContextValid(parent)) {
return new NoopSpan(parent);
} else if (
Expand Down
48 changes: 24 additions & 24 deletions packages/opentelemetry-core/test/context/B3MultiPropagator.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
defaultSetter,
SpanContext,
TraceFlags,
getExtractedSpanContext,
getActiveSpan,
setExtractedSpanContext,
} from '@opentelemetry/api';
import { ROOT_CONTEXT } from '@opentelemetry/context-base';
Expand Down Expand Up @@ -137,7 +137,7 @@ describe('B3MultiPropagator', () => {
carrier,
defaultGetter
);
const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();
assert.deepStrictEqual(extractedSpanContext, {
spanId: 'b7ad6b7169203331',
traceId: '0af7651916cd43dd8448eb211c80319c',
Expand All @@ -158,7 +158,7 @@ describe('B3MultiPropagator', () => {
carrier,
defaultGetter
);
const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();

assert.deepStrictEqual(extractedSpanContext, {
spanId: 'b7ad6b7169203331',
Expand All @@ -180,7 +180,7 @@ describe('B3MultiPropagator', () => {
carrier,
defaultGetter
);
const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();

assert.deepStrictEqual(extractedSpanContext, {
spanId: 'b7ad6b7169203331',
Expand All @@ -202,7 +202,7 @@ describe('B3MultiPropagator', () => {
carrier,
defaultGetter
);
const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();

assert.deepStrictEqual(extractedSpanContext, {
spanId: 'b7ad6b7169203331',
Expand All @@ -226,7 +226,7 @@ describe('B3MultiPropagator', () => {
carrier,
defaultGetter
);
const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();

assert.deepStrictEqual(extractedSpanContext, {
spanId: 'b7ad6b7169203331',
Expand All @@ -251,7 +251,7 @@ describe('B3MultiPropagator', () => {
carrier,
defaultGetter
);
const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();

assert.deepStrictEqual(extractedSpanContext, {
spanId: 'b7ad6b7169203331',
Expand All @@ -274,7 +274,7 @@ describe('B3MultiPropagator', () => {
carrier,
defaultGetter
);
const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();

assert.deepStrictEqual(extractedSpanContext, {
spanId: 'b7ad6b7169203331',
Expand All @@ -297,7 +297,7 @@ describe('B3MultiPropagator', () => {
carrier,
defaultGetter
);
const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();

assert.deepStrictEqual(extractedSpanContext, {
spanId: 'b7ad6b7169203331',
Expand All @@ -320,7 +320,7 @@ describe('B3MultiPropagator', () => {
carrier,
defaultGetter
);
const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();

assert.deepStrictEqual(extractedSpanContext, {
spanId: 'b7ad6b7169203331',
Expand All @@ -338,9 +338,9 @@ describe('B3MultiPropagator', () => {
it('should return undefined', () => {
carrier[X_B3_TRACE_ID] = undefined;
carrier[X_B3_SPAN_ID] = 'b7ad6b7169203331';
const context = getExtractedSpanContext(
const context = getActiveSpan(
b3Propagator.extract(ROOT_CONTEXT, carrier, defaultGetter)
);
)?.context();
assert.deepStrictEqual(context, undefined);
});
});
Expand All @@ -349,9 +349,9 @@ describe('B3MultiPropagator', () => {
it('should return undefined', () => {
carrier[X_B3_TRACE_ID] = '0af7651916cd43dd8448eb211c80319c';
carrier[X_B3_SPAN_ID] = undefined;
const context = getExtractedSpanContext(
const context = getActiveSpan(
b3Propagator.extract(ROOT_CONTEXT, carrier, defaultGetter)
);
)?.context();
assert.deepStrictEqual(context, undefined);
});
});
Expand All @@ -361,18 +361,18 @@ describe('B3MultiPropagator', () => {
carrier[X_B3_TRACE_ID] = '0af7651916cd43dd8448eb211c80319c';
carrier[X_B3_SPAN_ID] = 'b7ad6b7169203331';
carrier[X_B3_SAMPLED] = '2';
const context = getExtractedSpanContext(
const context = getActiveSpan(
b3Propagator.extract(ROOT_CONTEXT, carrier, defaultGetter)
);
)?.context();
assert.deepStrictEqual(context, undefined);
});
});

describe('AND b3 header is missing', () => {
it('should return undefined', () => {
const context = getExtractedSpanContext(
const context = getActiveSpan(
b3Propagator.extract(ROOT_CONTEXT, carrier, defaultGetter)
);
)?.context();
assert.deepStrictEqual(context, undefined);
});
});
Expand All @@ -381,9 +381,9 @@ describe('B3MultiPropagator', () => {
it('should return undefined', () => {
carrier[X_B3_TRACE_ID] = 'invalid!';
carrier[X_B3_SPAN_ID] = 'b7ad6b7169203331';
const context = getExtractedSpanContext(
const context = getActiveSpan(
b3Propagator.extract(ROOT_CONTEXT, carrier, defaultGetter)
);
)?.context();
assert.deepStrictEqual(context, undefined);
});
});
Expand All @@ -398,7 +398,7 @@ describe('B3MultiPropagator', () => {
carrier,
defaultGetter
);
const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();
assert.deepStrictEqual(extractedSpanContext, {
spanId: 'b7ad6b7169203331',
traceId: '0af7651916cd43dd8448eb211c80319c',
Expand Down Expand Up @@ -447,9 +447,9 @@ describe('B3MultiPropagator', () => {

Object.getOwnPropertyNames(testCases).forEach(testCase => {
carrier[X_B3_TRACE_ID] = testCases[testCase];
const extractedSpanContext = getExtractedSpanContext(
const extractedSpanContext = getActiveSpan(
b3Propagator.extract(ROOT_CONTEXT, carrier, defaultGetter)
);
)?.context();
assert.deepStrictEqual(extractedSpanContext, undefined, testCase);
});
});
Expand Down Expand Up @@ -485,7 +485,7 @@ describe('B3MultiPropagator', () => {
carrier,
defaultGetter
);
const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();

assert.deepStrictEqual(extractedSpanContext, {
spanId: 'b7ad6b7169203331',
Expand Down
8 changes: 4 additions & 4 deletions packages/opentelemetry-core/test/context/B3Propagator.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
defaultSetter,
SpanContext,
TraceFlags,
getExtractedSpanContext,
getActiveSpan,
setExtractedSpanContext,
} from '@opentelemetry/api';
import { ROOT_CONTEXT } from '@opentelemetry/context-base';
Expand Down Expand Up @@ -109,7 +109,7 @@ describe('B3Propagator', () => {
defaultGetter
);

const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();
assert.deepStrictEqual(extractedSpanContext, {
spanId: 'e457b5a2e4d86bd1',
traceId: '80f198ee56343ba864fe8b2a57d3eff7',
Expand All @@ -125,7 +125,7 @@ describe('B3Propagator', () => {
defaultGetter
);

const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();
assert.deepStrictEqual(extractedSpanContext, {
spanId: '6e0c63257de34c92',
traceId: 'd4cda95b652f4a1592b449d5929fda1b',
Expand All @@ -141,7 +141,7 @@ describe('B3Propagator', () => {
defaultGetter
);

const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();
assert.deepStrictEqual(extractedSpanContext, {
spanId: 'e457b5a2e4d86bd1',
traceId: '80f198ee56343ba864fe8b2a57d3eff7',
Expand Down
20 changes: 10 additions & 10 deletions packages/opentelemetry-core/test/context/B3SinglePropagator.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
TraceFlags,
INVALID_SPANID,
INVALID_TRACEID,
getExtractedSpanContext,
getActiveSpan,
setExtractedSpanContext,
} from '@opentelemetry/api';
import { ROOT_CONTEXT } from '@opentelemetry/context-base';
Expand Down Expand Up @@ -136,7 +136,7 @@ describe('B3SinglePropagator', () => {

const context = propagator.extract(ROOT_CONTEXT, carrier, defaultGetter);

const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();
assert.deepStrictEqual(extractedSpanContext, {
spanId: 'e457b5a2e4d86bd1',
traceId: '80f198ee56343ba864fe8b2a57d3eff7',
Expand All @@ -153,7 +153,7 @@ describe('B3SinglePropagator', () => {

const context = propagator.extract(ROOT_CONTEXT, carrier, defaultGetter);

const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();
assert.deepStrictEqual(extractedSpanContext, {
spanId: 'e457b5a2e4d86bd1',
traceId: '80f198ee56343ba864fe8b2a57d3eff7',
Expand All @@ -170,7 +170,7 @@ describe('B3SinglePropagator', () => {

const context = propagator.extract(ROOT_CONTEXT, carrier, defaultGetter);

const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();
assert.deepStrictEqual(extractedSpanContext, {
spanId: 'e457b5a2e4d86bd1',
traceId: '80f198ee56343ba864fe8b2a57d3eff7',
Expand All @@ -186,7 +186,7 @@ describe('B3SinglePropagator', () => {

const context = propagator.extract(ROOT_CONTEXT, carrier, defaultGetter);

const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();
assert.deepStrictEqual(extractedSpanContext, {
spanId: 'e457b5a2e4d86bd1',
traceId: '00000000000000004aaba1a52cf8ee09',
Expand All @@ -203,7 +203,7 @@ describe('B3SinglePropagator', () => {

const context = propagator.extract(ROOT_CONTEXT, carrier, defaultGetter);

const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();
assert.deepStrictEqual(extractedSpanContext, {
spanId: 'e457b5a2e4d86bd1',
traceId: '80f198ee56343ba864fe8b2a57d3eff7',
Expand All @@ -220,7 +220,7 @@ describe('B3SinglePropagator', () => {

const context = propagator.extract(ROOT_CONTEXT, carrier, defaultGetter);

const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();
assert.deepStrictEqual(undefined, extractedSpanContext);
});

Expand All @@ -231,7 +231,7 @@ describe('B3SinglePropagator', () => {

const context = propagator.extract(ROOT_CONTEXT, carrier, defaultGetter);

const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();
assert.deepStrictEqual(undefined, extractedSpanContext);
});

Expand All @@ -242,7 +242,7 @@ describe('B3SinglePropagator', () => {

const context = propagator.extract(ROOT_CONTEXT, carrier, defaultGetter);

const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();
assert.deepStrictEqual(undefined, extractedSpanContext);
});

Expand All @@ -253,7 +253,7 @@ describe('B3SinglePropagator', () => {

const context = propagator.extract(ROOT_CONTEXT, carrier, defaultGetter);

const extractedSpanContext = getExtractedSpanContext(context);
const extractedSpanContext = getActiveSpan(context)?.context();
assert.deepStrictEqual(undefined, extractedSpanContext);
});
});
Expand Down
Loading

0 comments on commit 1c27690

Please sign in to comment.