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.

Sign up for GitHub

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

Jump to bottom

fix(api): use active context as default in NoopTracer #3476

Merged
merged 7 commits into from
Dec 14, 2022
Merged

fix(api): use active context as default in NoopTracer #3476

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f49b92b
fix(api): use active context as default in NoopTracer
Flarna Dec 7, 2022
19538da
update PR link
Flarna Dec 7, 2022
1ea7858
Merge branch 'main' into propagation-only
Flarna Dec 7, 2022
987546f
fixup: correct link
Flarna Dec 7, 2022
90dac1e
Merge branch 'main' into propagation-only
Flarna Dec 10, 2022
32037bc
Merge remote-tracking branch 'upstream/main' into propagation-only
Flarna Dec 14, 2022
85e008d
remove unneeded assert
Flarna Dec 14, 2022
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
1 change: 1 addition & 0 deletions api/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file.

* fix(api): deprecate MetricAttributes and MetricAttributeValue [#3406](https://github.com/open-telemetry/opentelemetry-js/pull/3406) @blumamir
* test(api): disable module concatenation in tree-shaking test [#3409](https://github.com/open-telemetry/opentelemetry-js/pull/3409) @legendecas
* fix(api): use active context as default in NoopTracer [#3476] (https://github.com/open-telemetry/opentelemetry-js/pull/3476) @flarna
Flarna marked this conversation as resolved.
Show resolved Hide resolved

## [1.3.0](https://github.com/open-telemetry/opentelemetry-js-api/compare/v1.2.0...v1.3.0)

Expand Down
6 changes: 5 additions & 1 deletion api/src/trace/NoopTracer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ const contextApi = ContextAPI.getInstance();
*/
export class NoopTracer implements Tracer {
// startSpan starts a noop span.
startSpan(name: string, options?: SpanOptions, context?: Context): Span {
startSpan(
name: string,
options?: SpanOptions,
context = contextApi.active()
): Span {
const root = Boolean(options?.root);
if (root) {
return new NonRecordingSpan();
Expand Down
23 changes: 23 additions & 0 deletions api/test/common/noop-implementations/noop-tracer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@
*/

import * as assert from 'assert';
import * as sinon from 'sinon';
import {
context,
ROOT_CONTEXT,
Span,
SpanContext,
SpanKind,
Expand All @@ -27,6 +29,10 @@ import { NonRecordingSpan } from '../../../src/trace/NonRecordingSpan';
import { NoopTracer } from '../../../src/trace/NoopTracer';

describe('NoopTracer', () => {
afterEach(() => {
sinon.restore();
});

it('should not crash', () => {
const tracer = new NoopTracer();

Expand Down Expand Up @@ -58,6 +64,23 @@ describe('NoopTracer', () => {
assert(span.spanContext().traceFlags === parent.traceFlags);
});

it('should propagate valid spanContext on the span (from current context)', () => {
const tracer = new NoopTracer();
const parent: SpanContext = {
traceId: 'd4cda95b652f4a1592b449dd92ffda3b',
spanId: '6e0c63ffe4e34c42',
traceFlags: TraceFlags.NONE,
};

const ctx = trace.setSpanContext(ROOT_CONTEXT, parent);
const activeStub = sinon.stub(context, 'active');
activeStub.returns(ctx);
const span = tracer.startSpan('test-1');
assert(span.spanContext().traceId === parent.traceId);
assert(span.spanContext().spanId === parent.spanId);
assert(span.spanContext().traceFlags === parent.traceFlags);
});

it('should accept 2 to 4 args and start an active span', () => {
const tracer = new NoopTracer();
const name = 'span-name';
Expand Down
6 changes: 3 additions & 3 deletions api/test/common/proxy-implementations/proxy-tracer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,9 @@ describe('ProxyTracer', () => {
tracer.startSpan(name, options, ctx);

// Assert the proxy tracer has the full API of the NoopTracer
assert.strictEqual(
NoopTracer.prototype.startSpan.length,
ProxyTracer.prototype.startSpan.length
assert.ok(
Flarna marked this conversation as resolved.
Show resolved Hide resolved
ProxyTracer.prototype.startSpan.length >=
NoopTracer.prototype.startSpan.length
);
assert.deepStrictEqual(Object.getOwnPropertyNames(NoopTracer.prototype), [
'constructor',
Expand Down