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

feat: add startActiveSpan method to Tracer #2221

Merged
merged 27 commits into from
Jun 1, 2021
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
43283ab
feat: add startActiveSpan method to Tracer
May 21, 2021
f5c5a88
fix: add function overload signatures and remove unneeded typeof checks
May 21, 2021
fe3681e
chore: function overloads
May 21, 2021
3e69e73
chore: change var names
May 22, 2021
0a6307f
test: startActiveSpan in Tracer
May 22, 2021
6f9b8df
fix: startactiveSpan test
May 22, 2021
9f1521e
feat: add reset script
May 22, 2021
b1816b9
chore: organize imports
May 22, 2021
333932e
test: split combined test into 4 tests
May 24, 2021
ee7a305
test: startActiveSpan ensure that ctx sent is used
May 24, 2021
3ab1fe4
chore: organize imports
May 24, 2021
02b5dfd
chore: clarify todo
May 24, 2021
a1e5bdb
test: startActiveSpan in NodeTracer
May 24, 2021
0a00ddf
chore: put active span tests in own describe block
May 24, 2021
e6eb4f7
test: startActiveSpan in WebTracer
May 24, 2021
41ff0b1
chore: remove Tracer specific tests from Node and Web Tracers
May 24, 2021
374c5ea
fix: conflicts
May 26, 2021
63ec701
chore: remove git clean form reset script
May 26, 2021
0afe1ee
fix: conflicts
May 26, 2021
5532776
fix: use span.spanContext()
May 26, 2021
4219630
fix: adhere to breaking changes in new api
May 26, 2021
0335473
chore: only import api once
May 28, 2021
1944b4e
test: assert active spans are indeed active with help of TestStackCon…
May 28, 2021
ec33257
test: spy startSpan to ensure it is called with opts passed into star…
May 28, 2021
c395c81
fix: uncomment sinon spy
May 28, 2021
4eb9d6c
test: assert startSpan spy called with name when only name and fn pas…
May 28, 2021
d5bab9b
Merge branch 'main' into start-active-span
dyladan Jun 1, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@
"lint:examples": "eslint ./examples/**/*.js",
"lint:examples:fix": "eslint ./examples/**/*.js --fix",
"lint:markdown": "./node_modules/.bin/markdownlint $(git ls-files '*.md') -i ./CHANGELOG.md",
"lint:markdown:fix": "./node_modules/.bin/markdownlint $(git ls-files '*.md') -i ./CHANGELOG.md --fix"
"lint:markdown:fix": "./node_modules/.bin/markdownlint $(git ls-files '*.md') -i ./CHANGELOG.md --fix",
"reset": "lerna clean -y && rm -rf node_modules && npm i && npm run compile && npm run lint:fix"
},
"repository": "open-telemetry/opentelemetry-js",
"keywords": [
Expand Down
128 changes: 88 additions & 40 deletions packages/opentelemetry-tracing/src/Tracer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,46 +54,6 @@ export class Tracer implements api.Tracer {
this.instrumentationLibrary = instrumentationLibrary;
}

startActiveSpan<F extends (span: api.Span) => ReturnType<F>>(
name: string,
arg2: F | api.SpanOptions,
arg3?: F | api.Context,
arg4?: F
): ReturnType<F> | undefined {
let fn: F | undefined,
options: api.SpanOptions | undefined,
activeContext: api.Context | undefined;

if (arguments.length === 2 && typeof arg2 === 'function') {
fn = arg2;
} else if (
arguments.length === 3 &&
typeof arg2 === 'object' &&
typeof arg3 === 'function'
) {
options = arg2;
fn = arg3;
} else if (
arguments.length === 4 &&
typeof arg2 === 'object' &&
typeof arg3 === 'object' &&
typeof arg4 === 'function'
) {
options = arg2;
activeContext = arg3;
fn = arg4;
}

const parentContext = activeContext ?? api.context.active();
const span = this.startSpan(name, options, parentContext);
const contextWithSpanSet = api.trace.setSpan(parentContext, span);

if (fn) {
return api.context.with(contextWithSpanSet, fn, undefined, span);
}
return;
}

/**
* Starts a new Span or returns the default NoopSpan based on the sampling
* decision.
Expand Down Expand Up @@ -163,6 +123,94 @@ export class Tracer implements api.Tracer {
return span;
}

/**
* Starts a new {@link Span} and calls the given function passing it the
* created span as first argument.
* Additionally the new span gets set in context and this context is activated
* for the duration of the function call.
*
* @param name The name of the span
* @param [options] SpanOptions used for span creation
* @param [context] Context to use to extract parent
* @param fn function called in the context of the span and receives the newly created span as an argument
* @returns return value of fn
* @example
* const something = tracer.startActiveSpan('op', span => {
* try {
* do some work
* span.setStatus({code: SpanStatusCode.OK});
* return something;
* } catch (err) {
* span.setStatus({
* code: SpanStatusCode.ERROR,
* message: err.message,
* });
* throw err;
* } finally {
* span.end();
* }
* });
* @example
* const span = tracer.startActiveSpan('op', span => {
* try {
* do some work
* return span;
* } catch (err) {
* span.setStatus({
* code: SpanStatusCode.ERROR,
* message: err.message,
* });
* throw err;
* }
* });
* do some more work
* span.end();
*/
startActiveSpan<F extends (span: api.Span) => ReturnType<F>>(
dyladan marked this conversation as resolved.
Show resolved Hide resolved
name: string,
fn: F
): ReturnType<F>;
startActiveSpan<F extends (span: api.Span) => ReturnType<F>>(
name: string,
opts: api.SpanOptions,
fn: F
): ReturnType<F>;
startActiveSpan<F extends (span: api.Span) => ReturnType<F>>(
name: string,
opts: api.SpanOptions,
ctx: api.Context,
fn: F
): ReturnType<F>;
startActiveSpan<F extends (span: api.Span) => ReturnType<F>>(
name: string,
arg2?: F | api.SpanOptions,
arg3?: F | api.Context,
arg4?: F
): ReturnType<F> | undefined {
let opts: api.SpanOptions | undefined;
let ctx: api.Context | undefined;
let fn: F;

if (arguments.length < 2) {
return;
} else if (arguments.length === 2) {
fn = arg2 as F;
} else if (arguments.length === 3) {
opts = arg2 as api.SpanOptions | undefined;
fn = arg3 as F;
} else {
opts = arg2 as api.SpanOptions | undefined;
ctx = arg3 as api.Context | undefined;
fn = arg4 as F;
}

const parentContext = ctx ?? api.context.active();
const span = this.startSpan(name, opts, parentContext);
const contextWithSpanSet = api.trace.setSpan(parentContext, span);

return api.context.with(contextWithSpanSet, fn, undefined, span);
dyladan marked this conversation as resolved.
Show resolved Hide resolved
}

/** Returns the active {@link SpanLimits}. */
getSpanLimits(): SpanLimits {
return this._spanLimits;
Expand Down
82 changes: 79 additions & 3 deletions packages/opentelemetry-tracing/test/Tracer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,28 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import {
context,
createContextKey,
INVALID_TRACEID,
ROOT_CONTEXT,
Sampler,
SamplingDecision,
TraceFlags,
SpanContext,
trace,
TraceFlags
} from '@opentelemetry/api';
import { getSpan } from '@opentelemetry/api/build/src/trace/context-utils';
import {
AlwaysOffSampler,
AlwaysOnSampler,
InstrumentationLibrary,
suppressTracing,
suppressTracing
} from '@opentelemetry/core';
import * as assert from 'assert';
import { BasicTracerProvider, Span, Tracer } from '../src';
import { TestStackContextManager } from './export/TestStackContextManager';
import * as sinon from 'sinon';

describe('Tracer', () => {
const tracerProvider = new BasicTracerProvider();
Expand All @@ -49,7 +53,13 @@ describe('Tracer', () => {
}
}

beforeEach(() => {
const contextManager = new TestStackContextManager().enable();
context.setGlobalContextManager(contextManager);
});

afterEach(() => {
context.disable();
delete envSource.OTEL_TRACES_SAMPLER;
delete envSource.OTEL_TRACES_SAMPLER_ARG;
});
Expand Down Expand Up @@ -220,4 +230,70 @@ describe('Tracer', () => {
assert.strictEqual(context.traceFlags, TraceFlags.NONE);
span.end();
});

it('should start an active span with name and function args', () => {
const tracer = new Tracer(
{ name: 'default', version: '0.0.1' },
{ sampler: new TestSampler() },
tracerProvider
);

const spy = sinon.spy(tracer, "startSpan");

assert.strictEqual(tracer.startActiveSpan('my-span', span => {
try {
assert(spy.calledWith('my-span'))
assert.strictEqual(getSpan(context.active()), span)
naseemkullah marked this conversation as resolved.
Show resolved Hide resolved
return 1
} finally {
span.end();
}
}), 1);
});

it('should start an active span with name, options and function args', () => {

const tracer = new Tracer(
{ name: 'default', version: '0.0.1' },
{ sampler: new TestSampler() },
tracerProvider
);

const spy = sinon.spy(tracer, "startSpan");

assert.strictEqual(tracer.startActiveSpan('my-span', {attributes: {foo: 'bar'}}, span => {
try {
assert(spy.calledWith('my-span', {attributes: {foo: 'bar'}}))
assert.strictEqual(getSpan(context.active()), span)
return 1
} finally {
span.end();
}
}), 1);
});

it('should start an active span with name, options, context and function args', () => {
const tracer = new Tracer(
{ name: 'default', version: '0.0.1' },
{ sampler: new TestSampler() },
tracerProvider
);

const ctxKey = createContextKey('foo');

const ctx = context.active().setValue(ctxKey, 'bar')

const spy = sinon.spy(tracer, "startSpan");

assert.strictEqual(tracer.startActiveSpan('my-span', {attributes: {foo: 'bar'}}, ctx, span => {
try {
assert(spy.calledWith('my-span', {attributes: {foo: 'bar'}}, ctx))
assert.strictEqual(getSpan(context.active()), span)
assert.strictEqual(ctx.getValue(ctxKey), 'bar')
return 1
} finally {
span.end();
}
}), 1);
});
});