Skip to content

Commit

Permalink
chore: improve naming of span related context APIs (#1749)
Browse files Browse the repository at this point in the history
  • Loading branch information
Flarna authored Dec 21, 2020
1 parent 7e5da41 commit c1d4264
Show file tree
Hide file tree
Showing 26 changed files with 150 additions and 166 deletions.
34 changes: 14 additions & 20 deletions packages/opentelemetry-api/src/context/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,9 @@ import { Context, createContextKey } from '@opentelemetry/context-base';
import { Baggage, NoopSpan, Span, SpanContext } from '../';

/**
* Active span key
* span key
*/
const ACTIVE_SPAN_KEY = createContextKey(
'OpenTelemetry Context Key ACTIVE_SPAN'
);
const SPAN_KEY = createContextKey('OpenTelemetry Context Key SPAN');

/**
* Shared key for indicating if instrumentation should be suppressed beyond
Expand All @@ -38,49 +36,45 @@ const SUPPRESS_INSTRUMENTATION_KEY = createContextKey(
const BAGGAGE_KEY = createContextKey('OpenTelemetry Baggage Key');

/**
* Return the active span if one exists
* Return the span if one exists
*
* @param context context to get span from
*/
export function getActiveSpan(context: Context): Span | undefined {
return (context.getValue(ACTIVE_SPAN_KEY) as Span) || undefined;
export function getSpan(context: Context): Span | undefined {
return (context.getValue(SPAN_KEY) as Span) || undefined;
}

/**
* Set the active span on a context
* Set the span on a context
*
* @param context context to use as parent
* @param span span to set active
*/
export function setActiveSpan(context: Context, span: Span): Context {
return context.setValue(ACTIVE_SPAN_KEY, span);
export function setSpan(context: Context, span: Span): Context {
return context.setValue(SPAN_KEY, span);
}

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

/**
* Get the span context of the parent span if it exists,
* or the extracted span context if there is no active
* span.
* Get the span context of the span if it exists.
*
* @param context context to get values from
*/
export function getParentSpanContext(
context: Context
): SpanContext | undefined {
return getActiveSpan(context)?.context();
export function getSpanContext(context: Context): SpanContext | undefined {
return getSpan(context)?.context();
}

/**
Expand Down
4 changes: 2 additions & 2 deletions packages/opentelemetry-api/src/trace/NoopTracer.ts
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 { getParentSpanContext } from '../context/context';
import { getSpanContext } from '../context/context';

/**
* No-op implementations of {@link Tracer}.
Expand All @@ -35,7 +35,7 @@ export class NoopTracer implements Tracer {
return NOOP_SPAN;
}

const parentFromContext = context && getParentSpanContext(context);
const parentFromContext = context && getSpanContext(context);

if (
isSpanContext(parentFromContext) &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
SpanKind,
TraceFlags,
context,
setExtractedSpanContext,
setSpanContext,
} from '../../src';

describe('NoopTracer', () => {
Expand Down Expand Up @@ -70,7 +70,7 @@ describe('NoopTracer', () => {
const span = tracer.startSpan(
'test-1',
{},
setExtractedSpanContext(context.active(), parent)
setSpanContext(context.active(), parent)
);
assert(span.context().traceId === parent.traceId);
assert(span.context().spanId === parent.spanId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@

import {
Context,
getParentSpanContext,
setExtractedSpanContext,
getSpanContext,
setSpanContext,
SpanContext,
TextMapGetter,
TextMapPropagator,
Expand Down Expand Up @@ -86,7 +86,7 @@ export function parseTraceParent(traceParent: string): SpanContext | null {
*/
export class HttpTraceContext implements TextMapPropagator {
inject(context: Context, carrier: unknown, setter: TextMapSetter) {
const spanContext = getParentSpanContext(context);
const spanContext = getSpanContext(context);
if (!spanContext) return;

const traceParent = `${VERSION}-${spanContext.traceId}-${
Expand Down Expand Up @@ -126,7 +126,7 @@ export class HttpTraceContext implements TextMapPropagator {
typeof state === 'string' ? state : undefined
);
}
return setExtractedSpanContext(context, spanContext);
return setSpanContext(context, spanContext);
}

fields(): string[] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import {
Attributes,
Context,
getParentSpanContext,
getSpanContext,
Link,
Sampler,
SamplingResult,
Expand Down Expand Up @@ -67,7 +67,7 @@ export class ParentBasedSampler implements Sampler {
attributes: Attributes,
links: Link[]
): SamplingResult {
const parentContext = getParentSpanContext(context);
const parentContext = getSpanContext(context);

if (!parentContext) {
return this._root.shouldSample(
Expand Down
30 changes: 15 additions & 15 deletions packages/opentelemetry-core/test/context/HttpTraceContext.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import {
defaultTextMapSetter,
SpanContext,
TraceFlags,
getParentSpanContext,
setExtractedSpanContext,
getSpanContext,
setSpanContext,
} from '@opentelemetry/api';
import { ROOT_CONTEXT } from '@opentelemetry/context-base';
import * as assert from 'assert';
Expand Down Expand Up @@ -48,7 +48,7 @@ describe('HttpTraceContext', () => {
};

httpTraceContext.inject(
setExtractedSpanContext(ROOT_CONTEXT, spanContext),
setSpanContext(ROOT_CONTEXT, spanContext),
carrier,
defaultTextMapSetter
);
Expand All @@ -68,7 +68,7 @@ describe('HttpTraceContext', () => {
};

httpTraceContext.inject(
setExtractedSpanContext(ROOT_CONTEXT, spanContext),
setSpanContext(ROOT_CONTEXT, spanContext),
carrier,
defaultTextMapSetter
);
Expand All @@ -84,7 +84,7 @@ describe('HttpTraceContext', () => {
it('should extract context of a sampled span from carrier', () => {
carrier[TRACE_PARENT_HEADER] =
'00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01';
const extractedSpanContext = getParentSpanContext(
const extractedSpanContext = getSpanContext(
httpTraceContext.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter)
);

Expand All @@ -99,7 +99,7 @@ describe('HttpTraceContext', () => {
it('should extract context of a sampled span from carrier using a future version', () => {
carrier[TRACE_PARENT_HEADER] =
'cc-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01';
const extractedSpanContext = getParentSpanContext(
const extractedSpanContext = getSpanContext(
httpTraceContext.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter)
);

Expand All @@ -114,7 +114,7 @@ describe('HttpTraceContext', () => {
it('should extract context of a sampled span from carrier using a future version and future fields', () => {
carrier[TRACE_PARENT_HEADER] =
'cc-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01-what-the-future-will-be-like';
const extractedSpanContext = getParentSpanContext(
const extractedSpanContext = getSpanContext(
httpTraceContext.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter)
);

Expand All @@ -128,7 +128,7 @@ describe('HttpTraceContext', () => {

it('returns null if traceparent header is missing', () => {
assert.deepStrictEqual(
getParentSpanContext(
getSpanContext(
httpTraceContext.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter)
),
undefined
Expand All @@ -138,7 +138,7 @@ describe('HttpTraceContext', () => {
it('returns null if traceparent header is invalid', () => {
carrier[TRACE_PARENT_HEADER] = 'invalid!';
assert.deepStrictEqual(
getParentSpanContext(
getSpanContext(
httpTraceContext.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter)
),
undefined
Expand All @@ -151,7 +151,7 @@ describe('HttpTraceContext', () => {
'00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01-extra';

assert.deepStrictEqual(
getParentSpanContext(
getSpanContext(
httpTraceContext.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter)
),
undefined
Expand All @@ -162,7 +162,7 @@ describe('HttpTraceContext', () => {
carrier[TRACE_PARENT_HEADER] = [
'00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01',
];
const extractedSpanContext = getParentSpanContext(
const extractedSpanContext = getSpanContext(
httpTraceContext.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter)
);
assert.deepStrictEqual(extractedSpanContext, {
Expand All @@ -177,7 +177,7 @@ describe('HttpTraceContext', () => {
carrier[TRACE_PARENT_HEADER] =
'00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01';
carrier[TRACE_STATE_HEADER] = 'foo=bar,baz=qux';
const extractedSpanContext = getParentSpanContext(
const extractedSpanContext = getSpanContext(
httpTraceContext.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter)
);

Expand All @@ -195,7 +195,7 @@ describe('HttpTraceContext', () => {
carrier[TRACE_PARENT_HEADER] =
'00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01';
carrier[TRACE_STATE_HEADER] = ['foo=bar,baz=qux', 'quux=quuz'];
const extractedSpanContext = getParentSpanContext(
const extractedSpanContext = getSpanContext(
httpTraceContext.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter)
);
assert.deepStrictEqual(extractedSpanContext, {
Expand Down Expand Up @@ -249,7 +249,7 @@ describe('HttpTraceContext', () => {
Object.getOwnPropertyNames(testCases).forEach(testCase => {
carrier[TRACE_PARENT_HEADER] = testCases[testCase];

const extractedSpanContext = getParentSpanContext(
const extractedSpanContext = getSpanContext(
httpTraceContext.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter)
);
assert.deepStrictEqual(extractedSpanContext, undefined, testCase);
Expand All @@ -260,7 +260,7 @@ describe('HttpTraceContext', () => {
carrier[TRACE_PARENT_HEADER] =
'00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01';
carrier[TRACE_STATE_HEADER] = 'foo=1 \t , \t bar=2, \t baz=3 ';
const extractedSpanContext = getParentSpanContext(
const extractedSpanContext = getSpanContext(
httpTraceContext.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter)
);

Expand Down
10 changes: 5 additions & 5 deletions packages/opentelemetry-core/test/context/composite.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import {
defaultTextMapSetter,
TextMapPropagator,
SpanContext,
getParentSpanContext,
setExtractedSpanContext,
getSpanContext,
setSpanContext,
} from '@opentelemetry/api';
import { Context, ROOT_CONTEXT } from '@opentelemetry/context-base';
import * as assert from 'assert';
Expand Down Expand Up @@ -64,7 +64,7 @@ describe('Composite Propagator', () => {
traceFlags: 1,
traceState: new TraceState('foo=bar'),
};
ctxWithSpanContext = setExtractedSpanContext(ROOT_CONTEXT, spanContext);
ctxWithSpanContext = setSpanContext(ROOT_CONTEXT, spanContext);
});

it('should inject context using all configured propagators', () => {
Expand Down Expand Up @@ -113,7 +113,7 @@ describe('Composite Propagator', () => {
const composite = new CompositePropagator({
propagators: [new B3MultiPropagator(), new HttpTraceContext()],
});
const spanContext = getParentSpanContext(
const spanContext = getSpanContext(
composite.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter)
);

Expand All @@ -132,7 +132,7 @@ describe('Composite Propagator', () => {
const composite = new CompositePropagator({
propagators: [new ThrowingPropagator(), new HttpTraceContext()],
});
const spanContext = getParentSpanContext(
const spanContext = getSpanContext(
composite.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter)
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,7 @@ import * as assert from 'assert';
import * as api from '@opentelemetry/api';
import { AlwaysOnSampler } from '../../src/trace/sampler/AlwaysOnSampler';
import { ParentBasedSampler } from '../../src/trace/sampler/ParentBasedSampler';
import {
TraceFlags,
SpanKind,
setExtractedSpanContext,
} from '@opentelemetry/api';
import { TraceFlags, SpanKind, setSpanContext } from '@opentelemetry/api';
import { AlwaysOffSampler } from '../../src/trace/sampler/AlwaysOffSampler';
import { TraceIdRatioBasedSampler } from '../../src';

Expand Down Expand Up @@ -62,7 +58,7 @@ describe('ParentBasedSampler', () => {
};
assert.deepStrictEqual(
sampler.shouldSample(
setExtractedSpanContext(api.ROOT_CONTEXT, spanContext),
setSpanContext(api.ROOT_CONTEXT, spanContext),
traceId,
spanName,
SpanKind.CLIENT,
Expand Down Expand Up @@ -103,7 +99,7 @@ describe('ParentBasedSampler', () => {
};
assert.deepStrictEqual(
sampler.shouldSample(
setExtractedSpanContext(api.ROOT_CONTEXT, spanContext),
setSpanContext(api.ROOT_CONTEXT, spanContext),
traceId,
spanName,
SpanKind.CLIENT,
Expand Down
4 changes: 2 additions & 2 deletions packages/opentelemetry-instrumentation-http/src/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
SpanKind,
SpanOptions,
Status,
setActiveSpan,
setSpan,
SpanContext,
TraceFlags,
ROOT_CONTEXT,
Expand Down Expand Up @@ -540,7 +540,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
optionsParsed.headers = {};
}
propagation.inject(
setActiveSpan(context.active(), span),
setSpan(context.active(), span),
optionsParsed.headers
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
propagation,
Span as ISpan,
SpanKind,
getActiveSpan,
getSpan,
} from '@opentelemetry/api';
import { NoopLogger } from '@opentelemetry/core';
import { NodeTracerProvider } from '@opentelemetry/node';
Expand Down Expand Up @@ -695,9 +695,9 @@ describe('HttpInstrumentation', () => {
});

it('should not set span as active in context for outgoing request', done => {
assert.deepStrictEqual(getActiveSpan(context.active()), undefined);
assert.deepStrictEqual(getSpan(context.active()), undefined);
http.get(`${protocol}://${hostname}:${serverPort}/test`, res => {
assert.deepStrictEqual(getActiveSpan(context.active()), undefined);
assert.deepStrictEqual(getSpan(context.active()), undefined);
done();
});
});
Expand Down
Loading

0 comments on commit c1d4264

Please sign in to comment.