Skip to content

Commit

Permalink
chore: remove NoRecordingSpan (#1900)
Browse files Browse the repository at this point in the history
Co-authored-by: Daniel Dyla <[email protected]>
  • Loading branch information
Flarna and dyladan authored Feb 8, 2021
1 parent 2f0d612 commit cacbbdc
Show file tree
Hide file tree
Showing 9 changed files with 14 additions and 105 deletions.
1 change: 0 additions & 1 deletion packages/opentelemetry-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ export * from './context/propagation/HttpTraceContext';
export * from './context/propagation/types';
export * from './baggage/propagation/HttpBaggage';
export * from './platform';
export * from './trace/NoRecordingSpan';
export * from './trace/Plugin';
export * from './trace/sampler/AlwaysOffSampler';
export * from './trace/sampler/AlwaysOnSampler';
Expand Down
39 changes: 0 additions & 39 deletions packages/opentelemetry-core/src/trace/NoRecordingSpan.ts

This file was deleted.

33 changes: 0 additions & 33 deletions packages/opentelemetry-core/test/trace/NoRecordingSpan.test.ts

This file was deleted.

2 changes: 1 addition & 1 deletion packages/opentelemetry-instrumentation-http/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
"devDependencies": {
"@opentelemetry/context-async-hooks": "^0.16.0",
"@opentelemetry/context-base": "^0.16.0",
"@opentelemetry/core": "^0.16.0",
"@opentelemetry/node": "^0.16.0",
"@opentelemetry/tracing": "^0.16.0",
"@types/got": "9.6.11",
Expand Down Expand Up @@ -70,7 +71,6 @@
},
"dependencies": {
"@opentelemetry/api": "^0.16.0",
"@opentelemetry/core": "^0.16.0",
"@opentelemetry/instrumentation": "^0.16.0",
"@opentelemetry/semantic-conventions": "^0.16.0",
"semver": "^7.1.3"
Expand Down
13 changes: 3 additions & 10 deletions packages/opentelemetry-instrumentation-http/src/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,11 @@ import {
SpanOptions,
SpanStatus,
setSpan,
SpanContext,
TraceFlags,
ROOT_CONTEXT,
getSpan,
suppressInstrumentation,
NoopSpan,
} from '@opentelemetry/api';
import { NoRecordingSpan } from '@opentelemetry/core';
import type * as http from 'http';
import type * as https from 'https';
import { Socket } from 'net';
Expand Down Expand Up @@ -61,11 +59,6 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
/** keep track on spans not ended */
private readonly _spanNotEnded: WeakSet<Span> = new WeakSet<Span>();
private readonly _version = process.versions.node;
private readonly _emptySpanContext: SpanContext = {
traceId: '',
spanId: '',
traceFlags: TraceFlags.NONE,
};

constructor(config: HttpInstrumentationConfig & InstrumentationConfig = {}) {
super(
Expand Down Expand Up @@ -580,7 +573,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {

private _startHttpSpan(name: string, options: SpanOptions) {
/*
* If a parent is required but not present, we use a `NoRecordingSpan` to still
* If a parent is required but not present, we use a `NoopSpan` to still
* propagate context without recording it.
*/
const requireParent =
Expand All @@ -594,7 +587,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
if (requireParent === true && currentSpan === undefined) {
// TODO: Refactor this when a solution is found in
// https://github.com/open-telemetry/opentelemetry-specification/issues/530
span = new NoRecordingSpan(this._emptySpanContext);
span = new NoopSpan();
} else if (requireParent === true && currentSpan?.context().isRemote) {
span = currentSpan;
} else {
Expand Down
9 changes: 3 additions & 6 deletions packages/opentelemetry-node/test/NodeTracerProvider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,9 @@ import {
setSpan,
setSpanContext,
getSpan,
NoopSpan,
} from '@opentelemetry/api';
import {
AlwaysOnSampler,
AlwaysOffSampler,
NoRecordingSpan,
} from '@opentelemetry/core';
import { AlwaysOnSampler, AlwaysOffSampler } from '@opentelemetry/core';
import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks';
import { Span } from '@opentelemetry/tracing';
import { Resource, TELEMETRY_SDK_RESOURCE } from '@opentelemetry/resources';
Expand Down Expand Up @@ -124,7 +121,7 @@ describe('NodeTracerProvider', () => {
logger: new NoopLogger(),
});
const span = provider.getTracer('default').startSpan('my-span');
assert.ok(span instanceof NoRecordingSpan);
assert.ok(span instanceof NoopSpan);
assert.strictEqual(span.context().traceFlags, TraceFlags.NONE);
assert.strictEqual(span.isRecording(), false);
});
Expand Down
15 changes: 4 additions & 11 deletions packages/opentelemetry-plugin-http/src/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,13 @@ import {
SpanKind,
SpanOptions,
SpanStatus,
SpanContext,
TraceFlags,
setSpan,
ROOT_CONTEXT,
getSpan,
suppressInstrumentation,
NoopSpan,
} from '@opentelemetry/api';
import { BasePlugin, NoRecordingSpan } from '@opentelemetry/core';
import { BasePlugin } from '@opentelemetry/core';
import type {
ClientRequest,
IncomingMessage,
Expand Down Expand Up @@ -60,12 +59,6 @@ export class HttpPlugin extends BasePlugin<Http> {
/** keep track on spans not ended */
private readonly _spanNotEnded: WeakSet<Span>;

private readonly _emptySpanContext: SpanContext = {
traceId: '',
spanId: '',
traceFlags: TraceFlags.NONE,
};

constructor(readonly moduleName: string, readonly version: string) {
super(`@opentelemetry/plugin-${moduleName}`, VERSION);
// For now component is equal to moduleName but it can change in the future.
Expand Down Expand Up @@ -439,7 +432,7 @@ export class HttpPlugin extends BasePlugin<Http> {

private _startHttpSpan(name: string, options: SpanOptions) {
/*
* If a parent is required but not present, we use a `NoRecordingSpan` to still
* If a parent is required but not present, we use a `NoopSpan` to still
* propagate context without recording it.
*/
const requireParent =
Expand All @@ -453,7 +446,7 @@ export class HttpPlugin extends BasePlugin<Http> {
if (requireParent === true && currentSpan === undefined) {
// TODO: Refactor this when a solution is found in
// https://github.com/open-telemetry/opentelemetry-specification/issues/530
span = new NoRecordingSpan(plugin._emptySpanContext);
span = new NoopSpan();
} else if (requireParent === true && currentSpan?.context().isRemote) {
span = currentSpan;
} else {
Expand Down
3 changes: 1 addition & 2 deletions packages/opentelemetry-tracing/src/Tracer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import * as api from '@opentelemetry/api';
import {
ConsoleLogger,
InstrumentationLibrary,
NoRecordingSpan,
IdGenerator,
RandomIdGenerator,
sanitizeAttributes,
Expand Down Expand Up @@ -104,7 +103,7 @@ export class Tracer implements api.Tracer {
const spanContext = { traceId, spanId, traceFlags, traceState };
if (samplingResult.decision === api.SamplingDecision.NOT_RECORD) {
this.logger.debug('Recording is off, starting no recording span');
return new NoRecordingSpan(spanContext);
return new api.NoopSpan(spanContext);
}

const span = new Span(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ import {
TraceFlags,
ROOT_CONTEXT,
NoopLogger,
NoopSpan,
setSpan,
setSpanContext,
getSpan,
} from '@opentelemetry/api';
import {
AlwaysOnSampler,
AlwaysOffSampler,
NoRecordingSpan,
TraceState,
} from '@opentelemetry/core';
import { Resource } from '@opentelemetry/resources';
Expand Down Expand Up @@ -264,7 +264,7 @@ describe('BasicTracerProvider', () => {
logger: new NoopLogger(),
}).getTracer('default');
const span = tracer.startSpan('my-span');
assert.ok(span instanceof NoRecordingSpan);
assert.ok(span instanceof NoopSpan);
const context = span.context();
assert.ok(context.traceId.match(/[a-f0-9]{32}/));
assert.ok(context.spanId.match(/[a-f0-9]{16}/));
Expand Down

0 comments on commit cacbbdc

Please sign in to comment.