From 53d2b514e4772233a42a838303216df6ee93bb28 Mon Sep 17 00:00:00 2001 From: Andrew Date: Mon, 2 Nov 2020 20:33:59 +0200 Subject: [PATCH 1/5] chore: eslint - fix unused vars warnings, add precommit lint - part 1 (#1636) --- eslint.config.js | 1 + package.json | 1 + .../propagation/NoopTextMapPropagator.ts | 4 +-- .../src/metrics/NoopMeter.ts | 30 +++++++++---------- .../opentelemetry-api/src/trace/NoopLogger.ts | 8 ++--- .../opentelemetry-api/src/trace/NoopSpan.ts | 14 ++++----- .../opentelemetry-api/src/trace/NoopTracer.ts | 2 +- .../src/NoopContextManager.ts | 4 +-- .../src/common/ConsoleLogger.ts | 8 ++--- .../src/common/NoopLogger.ts | 8 ++--- .../src/platform/browser/timer-util.ts | 2 +- .../src/trace/sampler/AlwaysOffSampler.ts | 2 +- .../src/trace/sampler/AlwaysOnSampler.ts | 2 +- .../node/CollectorExporterNodeBase.ts | 2 +- .../src/export/Controller.ts | 2 +- .../src/export/NoopExporter.ts | 4 +-- .../opentelemetry-plugin-grpc/src/grpc.ts | 6 ++-- .../opentelemetry-plugin-http/src/http.ts | 4 +-- .../src/shim.ts | 2 +- .../src/NoopSpanProcessor.ts | 4 +-- .../src/export/BatchSpanProcessor.ts | 2 +- .../src/export/SimpleSpanProcessor.ts | 2 +- 22 files changed, 58 insertions(+), 56 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index 7c1a2ca8866..f590ddd90f2 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -23,6 +23,7 @@ module.exports = { "leadingUnderscore": "require" } ], + "@typescript-eslint/no-unused-vars": ["error", {"argsIgnorePattern": "^_", "args": "after-used"}], "@typescript-eslint/no-inferrable-types": ["error", { ignoreProperties: true }], "arrow-parens": ["error", "as-needed"], "prettier/prettier": ["error", { "singleQuote": true, "arrowParens": "avoid" }], diff --git a/package.json b/package.json index 68639e43a3f..7e085a7c7d7 100644 --- a/package.json +++ b/package.json @@ -61,6 +61,7 @@ }, "husky": { "hooks": { + "pre-commit": "lerna run --concurrency 1 --stream lint:fix --since HEAD --exclude-dependents", "commit-msg": "commitlint -E HUSKY_GIT_PARAMS" } } diff --git a/packages/opentelemetry-api/src/context/propagation/NoopTextMapPropagator.ts b/packages/opentelemetry-api/src/context/propagation/NoopTextMapPropagator.ts index ad94b5cfb21..e2ec6f1b92e 100644 --- a/packages/opentelemetry-api/src/context/propagation/NoopTextMapPropagator.ts +++ b/packages/opentelemetry-api/src/context/propagation/NoopTextMapPropagator.ts @@ -22,9 +22,9 @@ import { TextMapPropagator } from './TextMapPropagator'; */ export class NoopTextMapPropagator implements TextMapPropagator { /** Noop inject function does nothing */ - inject(context: Context, carrier: unknown): void {} + inject(_context: Context, _carrier: unknown): void {} /** Noop extract function does nothing and returns the input context */ - extract(context: Context, carrier: unknown): Context { + extract(context: Context, _carrier: unknown): Context { return context; } fields(): string[] { diff --git a/packages/opentelemetry-api/src/metrics/NoopMeter.ts b/packages/opentelemetry-api/src/metrics/NoopMeter.ts index 765a0fd8ae3..d59d658b1e3 100644 --- a/packages/opentelemetry-api/src/metrics/NoopMeter.ts +++ b/packages/opentelemetry-api/src/metrics/NoopMeter.ts @@ -48,7 +48,7 @@ export class NoopMeter implements Meter { * @param name the name of the metric. * @param [options] the metric options. */ - createValueRecorder(name: string, options?: MetricOptions): ValueRecorder { + createValueRecorder(_name: string, _options?: MetricOptions): ValueRecorder { return NOOP_VALUE_RECORDER_METRIC; } @@ -57,7 +57,7 @@ export class NoopMeter implements Meter { * @param name the name of the metric. * @param [options] the metric options. */ - createCounter(name: string, options?: MetricOptions): Counter { + createCounter(_name: string, _options?: MetricOptions): Counter { return NOOP_COUNTER_METRIC; } @@ -66,7 +66,7 @@ export class NoopMeter implements Meter { * @param name the name of the metric. * @param [options] the metric options. */ - createUpDownCounter(name: string, options?: MetricOptions): UpDownCounter { + createUpDownCounter(_name: string, _options?: MetricOptions): UpDownCounter { return NOOP_COUNTER_METRIC; } @@ -77,9 +77,9 @@ export class NoopMeter implements Meter { * @param [callback] the value observer callback */ createValueObserver( - name: string, - options?: MetricOptions, - callback?: (observerResult: ObserverResult) => void + _name: string, + _options?: MetricOptions, + _callback?: (observerResult: ObserverResult) => void ): ValueObserver { return NOOP_VALUE_OBSERVER_METRIC; } @@ -90,8 +90,8 @@ export class NoopMeter implements Meter { * @param callback the batch observer callback */ createBatchObserver( - name: string, - callback: (batchObserverResult: BatchObserverResult) => void + _name: string, + _callback: (batchObserverResult: BatchObserverResult) => void ): BatchObserver { return NOOP_BATCH_OBSERVER_METRIC; } @@ -111,7 +111,7 @@ export class NoopMetric implements UnboundMetric { * @param labels key-values pairs that are associated with a specific metric * that you want to record. */ - bind(labels: Labels): T { + bind(_labels: Labels): T { return this._instrument; } @@ -119,7 +119,7 @@ export class NoopMetric implements UnboundMetric { * Removes the Binding from the metric, if it is present. * @param labels key-values pairs that are associated with a specific metric. */ - unbind(labels: Labels): void { + unbind(_labels: Labels): void { return; } @@ -163,23 +163,23 @@ export class NoopBatchObserverMetric implements BatchObserver {} export class NoopBoundCounter implements BoundCounter { - add(value: number): void { + add(_value: number): void { return; } } export class NoopBoundValueRecorder implements BoundValueRecorder { record( - value: number, - correlationContext?: CorrelationContext, - spanContext?: SpanContext + _value: number, + _correlationContext?: CorrelationContext, + _spanContext?: SpanContext ): void { return; } } export class NoopBoundBaseObserver implements BoundBaseObserver { - update(value: number) {} + update(_value: number) {} } export const NOOP_METER = new NoopMeter(); diff --git a/packages/opentelemetry-api/src/trace/NoopLogger.ts b/packages/opentelemetry-api/src/trace/NoopLogger.ts index dae032e0ff1..1fb8b184d07 100644 --- a/packages/opentelemetry-api/src/trace/NoopLogger.ts +++ b/packages/opentelemetry-api/src/trace/NoopLogger.ts @@ -19,14 +19,14 @@ import { Logger } from '../common/Logger'; /** No-op implementation of Logger */ export class NoopLogger implements Logger { // By default does nothing - debug(message: string, ...args: unknown[]) {} + debug(_message: string, ..._args: unknown[]) {} // By default does nothing - error(message: string, ...args: unknown[]) {} + error(_message: string, ..._args: unknown[]) {} // By default does nothing - warn(message: string, ...args: unknown[]) {} + warn(_message: string, ..._args: unknown[]) {} // By default does nothing - info(message: string, ...args: unknown[]) {} + info(_message: string, ..._args: unknown[]) {} } diff --git a/packages/opentelemetry-api/src/trace/NoopSpan.ts b/packages/opentelemetry-api/src/trace/NoopSpan.ts index 5f599dfda99..0c175ed44c2 100644 --- a/packages/opentelemetry-api/src/trace/NoopSpan.ts +++ b/packages/opentelemetry-api/src/trace/NoopSpan.ts @@ -38,32 +38,32 @@ export class NoopSpan implements Span { } // By default does nothing - setAttribute(key: string, value: unknown): this { + setAttribute(_key: string, _value: unknown): this { return this; } // By default does nothing - setAttributes(attributes: Attributes): this { + setAttributes(_attributes: Attributes): this { return this; } // By default does nothing - addEvent(name: string, attributes?: Attributes): this { + addEvent(_name: string, _attributes?: Attributes): this { return this; } // By default does nothing - setStatus(status: Status): this { + setStatus(_status: Status): this { return this; } // By default does nothing - updateName(name: string): this { + updateName(_name: string): this { return this; } // By default does nothing - end(endTime?: TimeInput): void {} + end(_endTime?: TimeInput): void {} // isRecording always returns false for noopSpan. isRecording(): boolean { @@ -71,7 +71,7 @@ export class NoopSpan implements Span { } // By default does nothing - recordException(exception: Exception, time?: TimeInput): void {} + recordException(_exception: Exception, _time?: TimeInput): void {} } export const NOOP_SPAN = new NoopSpan(); diff --git a/packages/opentelemetry-api/src/trace/NoopTracer.ts b/packages/opentelemetry-api/src/trace/NoopTracer.ts index 7f61200d7e2..cdc2cbd43d9 100644 --- a/packages/opentelemetry-api/src/trace/NoopTracer.ts +++ b/packages/opentelemetry-api/src/trace/NoopTracer.ts @@ -51,7 +51,7 @@ export class NoopTracer implements Tracer { return fn(); } - bind(target: T, span?: Span): T { + bind(target: T, _span?: Span): T { return target; } } diff --git a/packages/opentelemetry-context-base/src/NoopContextManager.ts b/packages/opentelemetry-context-base/src/NoopContextManager.ts index fd2e9f2121e..cb7fdd0de6e 100644 --- a/packages/opentelemetry-context-base/src/NoopContextManager.ts +++ b/packages/opentelemetry-context-base/src/NoopContextManager.ts @@ -23,13 +23,13 @@ export class NoopContextManager implements types.ContextManager { } with ReturnType>( - context: types.Context, + _context: types.Context, fn: T ): ReturnType { return fn(); } - bind(target: T, context?: types.Context): T { + bind(target: T, _context?: types.Context): T { return target; } diff --git a/packages/opentelemetry-core/src/common/ConsoleLogger.ts b/packages/opentelemetry-core/src/common/ConsoleLogger.ts index d0ee72eaffb..5dfe9e59882 100644 --- a/packages/opentelemetry-core/src/common/ConsoleLogger.ts +++ b/packages/opentelemetry-core/src/common/ConsoleLogger.ts @@ -42,11 +42,11 @@ export class ConsoleLogger implements Logger { } } - debug(message: string, ...args: unknown[]) {} + debug(_message: string, ..._args: unknown[]) {} - error(message: string, ...args: unknown[]) {} + error(_message: string, ..._args: unknown[]) {} - warn(message: string, ...args: unknown[]) {} + warn(_message: string, ..._args: unknown[]) {} - info(message: string, ...args: unknown[]) {} + info(_message: string, ..._args: unknown[]) {} } diff --git a/packages/opentelemetry-core/src/common/NoopLogger.ts b/packages/opentelemetry-core/src/common/NoopLogger.ts index ad8cc8b639e..b3db42c994d 100644 --- a/packages/opentelemetry-core/src/common/NoopLogger.ts +++ b/packages/opentelemetry-core/src/common/NoopLogger.ts @@ -19,14 +19,14 @@ import { Logger } from '@opentelemetry/api'; /** No-op implementation of Logger */ export class NoopLogger implements Logger { // By default does nothing - debug(message: string, ...args: unknown[]) {} + debug(_message: string, ..._args: unknown[]) {} // By default does nothing - error(message: string, ...args: unknown[]) {} + error(_message: string, ..._args: unknown[]) {} // By default does nothing - warn(message: string, ...args: unknown[]) {} + warn(_message: string, ..._args: unknown[]) {} // By default does nothing - info(message: string, ...args: unknown[]) {} + info(_message: string, ..._args: unknown[]) {} } diff --git a/packages/opentelemetry-core/src/platform/browser/timer-util.ts b/packages/opentelemetry-core/src/platform/browser/timer-util.ts index 2798b6c0dcb..2ed348b5b1e 100644 --- a/packages/opentelemetry-core/src/platform/browser/timer-util.ts +++ b/packages/opentelemetry-core/src/platform/browser/timer-util.ts @@ -13,4 +13,4 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -export function unrefTimer(timer: number): void {} +export function unrefTimer(_timer: number): void {} diff --git a/packages/opentelemetry-core/src/trace/sampler/AlwaysOffSampler.ts b/packages/opentelemetry-core/src/trace/sampler/AlwaysOffSampler.ts index 9f49a1f4c7d..67c169efe9a 100644 --- a/packages/opentelemetry-core/src/trace/sampler/AlwaysOffSampler.ts +++ b/packages/opentelemetry-core/src/trace/sampler/AlwaysOffSampler.ts @@ -25,6 +25,6 @@ export class AlwaysOffSampler implements Sampler { } toString(): string { - return `AlwaysOffSampler`; + return 'AlwaysOffSampler'; } } diff --git a/packages/opentelemetry-core/src/trace/sampler/AlwaysOnSampler.ts b/packages/opentelemetry-core/src/trace/sampler/AlwaysOnSampler.ts index 12e912cca52..5d15b88526f 100644 --- a/packages/opentelemetry-core/src/trace/sampler/AlwaysOnSampler.ts +++ b/packages/opentelemetry-core/src/trace/sampler/AlwaysOnSampler.ts @@ -25,6 +25,6 @@ export class AlwaysOnSampler implements Sampler { } toString(): string { - return `AlwaysOnSampler`; + return 'AlwaysOnSampler'; } } diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporterNodeBase.ts b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporterNodeBase.ts index bcca2508785..e1f2831f6cf 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporterNodeBase.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporterNodeBase.ts @@ -42,7 +42,7 @@ export abstract class CollectorExporterNodeBase< parseHeaders(config.headers, this.logger) || this.DEFAULT_HEADERS; } - onInit(config: CollectorExporterConfigBase): void { + onInit(_config: CollectorExporterConfigBase): void { this._isShutdown = false; } diff --git a/packages/opentelemetry-metrics/src/export/Controller.ts b/packages/opentelemetry-metrics/src/export/Controller.ts index 662218c10aa..d160fa73047 100644 --- a/packages/opentelemetry-metrics/src/export/Controller.ts +++ b/packages/opentelemetry-metrics/src/export/Controller.ts @@ -49,7 +49,7 @@ export class PushController extends Controller { private async _collect(): Promise { await this._meter.collect(); - return new Promise((resolve, reject) => { + return new Promise(resolve => { this._exporter.export( this._meter.getBatcher().checkPointSet(), result => { diff --git a/packages/opentelemetry-metrics/src/export/NoopExporter.ts b/packages/opentelemetry-metrics/src/export/NoopExporter.ts index 11e24d2464c..8e279833f2d 100644 --- a/packages/opentelemetry-metrics/src/export/NoopExporter.ts +++ b/packages/opentelemetry-metrics/src/export/NoopExporter.ts @@ -20,8 +20,8 @@ import { ExportResult } from '@opentelemetry/core'; export class NoopExporter implements MetricExporter { // By default does nothing export( - metrics: MetricRecord[], - resultCallback: (result: ExportResult) => void + _metrics: MetricRecord[], + _resultCallback: (result: ExportResult) => void ): void {} // By default does nothing diff --git a/packages/opentelemetry-plugin-grpc/src/grpc.ts b/packages/opentelemetry-plugin-grpc/src/grpc.ts index bc185dc777d..8a8fa5c0730 100644 --- a/packages/opentelemetry-plugin-grpc/src/grpc.ts +++ b/packages/opentelemetry-plugin-grpc/src/grpc.ts @@ -342,8 +342,8 @@ export class GrpcPlugin extends BasePlugin { return function makeClientConstructor( this: typeof grpcTypes.Client, methods: { [key: string]: { originalName?: string } }, - serviceName: string, - options: grpcTypes.GenericClientOptions + _serviceName: string, + _options: grpcTypes.GenericClientOptions ) { const client = original.apply(this, arguments as any); shimmer.massWrap( @@ -421,7 +421,7 @@ export class GrpcPlugin extends BasePlugin { function patchedCallback( span: Span, callback: SendUnaryDataCallback, - metadata: grpcTypes.Metadata + _metadata: grpcTypes.Metadata ) { const wrappedFn = (err: grpcTypes.ServiceError, res: any) => { if (err) { diff --git a/packages/opentelemetry-plugin-http/src/http.ts b/packages/opentelemetry-plugin-http/src/http.ts index 2f18916ea49..200a9b46971 100644 --- a/packages/opentelemetry-plugin-http/src/http.ts +++ b/packages/opentelemetry-plugin-http/src/http.ts @@ -155,7 +155,7 @@ export class HttpPlugin extends BasePlugin { ...args: HttpRequestArgs ) => ClientRequest ) { - return (original: Func): Func => { + return (_original: Func): Func => { // Re-implement http.get. This needs to be done (instead of using // getPatchOutgoingRequestFunction to patch it) because we need to // set the trace context header before the returned ClientRequest is @@ -325,7 +325,7 @@ export class HttpPlugin extends BasePlugin { const originalEnd = response.end; response.end = function ( this: ServerResponse, - ...args: ResponseEndArgs + ..._args: ResponseEndArgs ) { response.end = originalEnd; // Cannot pass args of type ResponseEndArgs, diff --git a/packages/opentelemetry-shim-opentracing/src/shim.ts b/packages/opentelemetry-shim-opentracing/src/shim.ts index 5b1364308a8..8f3ac08a16f 100644 --- a/packages/opentelemetry-shim-opentracing/src/shim.ts +++ b/packages/opentelemetry-shim-opentracing/src/shim.ts @@ -293,7 +293,7 @@ export class SpanShim extends opentracing.Span { * Logs a set of key value pairs. Since OpenTelemetry only supports events, * the KV pairs are used as attributes on an event named "log". */ - log(keyValuePairs: Attributes, timestamp?: number): this { + log(keyValuePairs: Attributes, _timestamp?: number): this { // @todo: Handle timestamp this._span.addEvent('log', keyValuePairs); return this; diff --git a/packages/opentelemetry-tracing/src/NoopSpanProcessor.ts b/packages/opentelemetry-tracing/src/NoopSpanProcessor.ts index 0d0e9c99e8d..6623803e857 100644 --- a/packages/opentelemetry-tracing/src/NoopSpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/NoopSpanProcessor.ts @@ -21,8 +21,8 @@ import { SpanProcessor } from './SpanProcessor'; /** No-op implementation of SpanProcessor */ export class NoopSpanProcessor implements SpanProcessor { - onStart(span: Span, context: Context): void {} - onEnd(span: ReadableSpan): void {} + onStart(_span: Span, _context: Context): void {} + onEnd(_span: ReadableSpan): void {} shutdown(): Promise { return Promise.resolve(); } diff --git a/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts b/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts index bfccb6665cf..a2373ae0381 100644 --- a/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/export/BatchSpanProcessor.ts @@ -59,7 +59,7 @@ export class BatchSpanProcessor implements SpanProcessor { } // does nothing. - onStart(span: Span): void {} + onStart(_span: Span): void {} onEnd(span: ReadableSpan): void { if (this._isShutdown) { diff --git a/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts b/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts index b915cbf39a9..fbf7cab8723 100644 --- a/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts +++ b/packages/opentelemetry-tracing/src/export/SimpleSpanProcessor.ts @@ -39,7 +39,7 @@ export class SimpleSpanProcessor implements SpanProcessor { } // does nothing. - onStart(span: Span): void {} + onStart(_span: Span): void {} onEnd(span: ReadableSpan): void { if (this._isShutdown) { From c4cdc4c26d5b5a77b6d84ce572c8adb4fedf3648 Mon Sep 17 00:00:00 2001 From: Valentin Marchaud Date: Mon, 2 Nov 2020 19:47:08 +0100 Subject: [PATCH 2/5] feat: isRecording return false if span is ended #1633 (#1642) Co-authored-by: Daniel Dyla --- packages/opentelemetry-tracing/src/Span.ts | 2 +- .../opentelemetry-tracing/test/Span.test.ts | 33 +++++++++++++------ 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/packages/opentelemetry-tracing/src/Span.ts b/packages/opentelemetry-tracing/src/Span.ts index 61d0869b6a8..5fdc21e591d 100644 --- a/packages/opentelemetry-tracing/src/Span.ts +++ b/packages/opentelemetry-tracing/src/Span.ts @@ -191,7 +191,7 @@ export class Span implements api.Span, ReadableSpan { } isRecording(): boolean { - return true; + return this._ended === false; } recordException(exception: api.Exception, time: api.TimeInput = hrTime()) { diff --git a/packages/opentelemetry-tracing/test/Span.test.ts b/packages/opentelemetry-tracing/test/Span.test.ts index 91d67d86707..07c7f2119db 100644 --- a/packages/opentelemetry-tracing/test/Span.test.ts +++ b/packages/opentelemetry-tracing/test/Span.test.ts @@ -186,16 +186,29 @@ describe('Span', () => { span.end(); }); - it('should return true when isRecording:true', () => { - const span = new Span( - tracer, - ROOT_CONTEXT, - name, - spanContext, - SpanKind.CLIENT - ); - assert.ok(span.isRecording()); - span.end(); + describe('isRecording', () => { + it('should return true when span is not ended', () => { + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.CLIENT + ); + assert.ok(span.isRecording()); + span.end(); + }); + it('should return false when span is ended', () => { + const span = new Span( + tracer, + ROOT_CONTEXT, + name, + spanContext, + SpanKind.CLIENT + ); + span.end(); + assert.ok(span.isRecording() === false); + }); }); it('should set an attribute', () => { From 18b49af9571775c4aaabb7eaa1cb4563ffedb1d9 Mon Sep 17 00:00:00 2001 From: Daniel Dyla Date: Mon, 2 Nov 2020 15:18:34 -0500 Subject: [PATCH 3/5] chore: remove explicit parent option (#1612) --- .../opentelemetry-api/src/api/global-utils.ts | 2 +- .../opentelemetry-api/src/trace/NoopTracer.ts | 11 +++-- .../src/trace/SpanOptions.ts | 19 ++------- .../noop-implementations/noop-tracer.test.ts | 13 ------ .../test/NodeTracerProvider.test.ts | 37 ++++++++++------- .../opentelemetry-plugin-fetch/src/fetch.ts | 12 ++++-- packages/opentelemetry-tracing/src/Tracer.ts | 11 +---- .../test/BasicTracerProvider.test.ts | 40 +------------------ 8 files changed, 45 insertions(+), 100 deletions(-) diff --git a/packages/opentelemetry-api/src/api/global-utils.ts b/packages/opentelemetry-api/src/api/global-utils.ts index 87791f2817a..297836e009e 100644 --- a/packages/opentelemetry-api/src/api/global-utils.ts +++ b/packages/opentelemetry-api/src/api/global-utils.ts @@ -65,4 +65,4 @@ export function makeGetter( * version. If the global API is not compatible with the API package * attempting to get it, a NOOP API implementation will be returned. */ -export const API_BACKWARDS_COMPATIBILITY_VERSION = 1; +export const API_BACKWARDS_COMPATIBILITY_VERSION = 2; diff --git a/packages/opentelemetry-api/src/trace/NoopTracer.ts b/packages/opentelemetry-api/src/trace/NoopTracer.ts index cdc2cbd43d9..f76a73ee548 100644 --- a/packages/opentelemetry-api/src/trace/NoopTracer.ts +++ b/packages/opentelemetry-api/src/trace/NoopTracer.ts @@ -30,11 +30,14 @@ export class NoopTracer implements Tracer { // startSpan starts a noop span. startSpan(name: string, options?: SpanOptions, context?: Context): Span { - const parent = options?.parent; + const root = Boolean(options?.root); + if (root) { + return NOOP_SPAN; + } + const parentFromContext = context && getActiveSpan(context)?.context(); - if (isSpanContext(parent) && isSpanContextValid(parent)) { - return new NoopSpan(parent); - } else if ( + + if ( isSpanContext(parentFromContext) && isSpanContextValid(parentFromContext) ) { diff --git a/packages/opentelemetry-api/src/trace/SpanOptions.ts b/packages/opentelemetry-api/src/trace/SpanOptions.ts index e20f243eacc..3c374044ae0 100644 --- a/packages/opentelemetry-api/src/trace/SpanOptions.ts +++ b/packages/opentelemetry-api/src/trace/SpanOptions.ts @@ -17,8 +17,6 @@ import { Attributes } from './attributes'; import { Link } from './link'; import { SpanKind } from './span_kind'; -import { Span } from './span'; -import { SpanContext } from './span_context'; /** * Options needed for span creation @@ -36,20 +34,9 @@ export interface SpanOptions { /** {@link Link}s span to other spans */ links?: Link[]; - /** - * This option is NOT RECOMMENDED for normal use and should ONLY be used - * if your application manages context manually without the global context - * manager, or you are trying to override the parent extracted from context. - * - * A parent `SpanContext` (or `Span`, for convenience) that the newly-started - * span will be the child of. This overrides the parent span extracted from - * the currently active context. - * - * A null value here should prevent the SDK from extracting a parent from - * the current context, forcing the new span to be a root span. - */ - parent?: Span | SpanContext | null; - /** A manually specified start time for the created `Span` object. */ startTime?: number; + + /** The new span should be a root span. (Ignore parent from context). */ + root?: boolean; } diff --git a/packages/opentelemetry-api/test/noop-implementations/noop-tracer.test.ts b/packages/opentelemetry-api/test/noop-implementations/noop-tracer.test.ts index 3f68c8a14c9..bfc06b81335 100644 --- a/packages/opentelemetry-api/test/noop-implementations/noop-tracer.test.ts +++ b/packages/opentelemetry-api/test/noop-implementations/noop-tracer.test.ts @@ -60,19 +60,6 @@ describe('NoopTracer', () => { return patchedFn(); }); - it('should propagate valid spanContext on the span (from parent)', () => { - const tracer = new NoopTracer(); - const parent: SpanContext = { - traceId: 'd4cda95b652f4a1592b449d5929fda1b', - spanId: '6e0c63257de34c92', - traceFlags: TraceFlags.NONE, - }; - const span = tracer.startSpan('test-1', { parent }); - assert(span.context().traceId === parent.traceId); - assert(span.context().spanId === parent.spanId); - assert(span.context().traceFlags === parent.traceFlags); - }); - it('should propagate valid spanContext on the span (from context)', () => { const tracer = new NoopTracer(); const parent: SpanContext = { diff --git a/packages/opentelemetry-node/test/NodeTracerProvider.test.ts b/packages/opentelemetry-node/test/NodeTracerProvider.test.ts index debbca3dcec..5fcc81e1b8c 100644 --- a/packages/opentelemetry-node/test/NodeTracerProvider.test.ts +++ b/packages/opentelemetry-node/test/NodeTracerProvider.test.ts @@ -14,7 +14,12 @@ * limitations under the License. */ -import { context, TraceFlags, setActiveSpan } from '@opentelemetry/api'; +import { + context, + TraceFlags, + setActiveSpan, + setExtractedSpanContext, +} from '@opentelemetry/api'; import { AlwaysOnSampler, AlwaysOffSampler, @@ -26,7 +31,7 @@ import { Span } from '@opentelemetry/tracing'; import { Resource, TELEMETRY_SDK_RESOURCE } from '@opentelemetry/resources'; import * as assert from 'assert'; import * as path from 'path'; -import { ContextManager } from '@opentelemetry/context-base'; +import { ContextManager, ROOT_CONTEXT } from '@opentelemetry/context-base'; import { NodeTracerProvider } from '../src/NodeTracerProvider'; const sleep = (time: number) => @@ -160,15 +165,15 @@ describe('NodeTracerProvider', () => { logger: new NoopLogger(), }); - const sampledParent = provider - .getTracer('default') - .startSpan('not-sampled-span', { - parent: { - traceId: 'd4cda95b652f4a1592b449d5929fda1b', - spanId: '6e0c63257de34c92', - traceFlags: TraceFlags.NONE, - }, - }); + const sampledParent = provider.getTracer('default').startSpan( + 'not-sampled-span', + {}, + setExtractedSpanContext(ROOT_CONTEXT, { + traceId: 'd4cda95b652f4a1592b449d5929fda1b', + spanId: '6e0c63257de34c92', + traceFlags: TraceFlags.NONE, + }) + ); assert.ok(sampledParent instanceof Span); assert.strictEqual( sampledParent.context().traceFlags, @@ -176,9 +181,13 @@ describe('NodeTracerProvider', () => { ); assert.strictEqual(sampledParent.isRecording(), true); - const span = provider.getTracer('default').startSpan('child-span', { - parent: sampledParent, - }); + const span = provider + .getTracer('default') + .startSpan( + 'child-span', + {}, + setActiveSpan(ROOT_CONTEXT, sampledParent) + ); assert.ok(span instanceof Span); assert.strictEqual(span.context().traceFlags, TraceFlags.SAMPLED); assert.strictEqual(span.isRecording(), true); diff --git a/packages/opentelemetry-plugin-fetch/src/fetch.ts b/packages/opentelemetry-plugin-fetch/src/fetch.ts index 11815c899ad..20b28ea833b 100644 --- a/packages/opentelemetry-plugin-fetch/src/fetch.ts +++ b/packages/opentelemetry-plugin-fetch/src/fetch.ts @@ -21,6 +21,7 @@ import * as web from '@opentelemetry/web'; import { AttributeNames } from './enums/AttributeNames'; import { FetchError, FetchResponse, SpanData } from './types'; import { VERSION } from './version'; +import { setActiveSpan } from '@opentelemetry/api'; // how long to wait for observer to collect information about resources // this is needed as event "load" is called before observer @@ -63,10 +64,13 @@ export class FetchPlugin extends core.BasePlugin> { span: api.Span, corsPreFlightRequest: PerformanceResourceTiming ): void { - const childSpan = this._tracer.startSpan('CORS Preflight', { - parent: span, - startTime: corsPreFlightRequest[web.PerformanceTimingNames.FETCH_START], - }); + const childSpan = this._tracer.startSpan( + 'CORS Preflight', + { + startTime: corsPreFlightRequest[web.PerformanceTimingNames.FETCH_START], + }, + setActiveSpan(api.context.active(), span) + ); web.addSpanNetworkEvents(childSpan, corsPreFlightRequest); childSpan.end( corsPreFlightRequest[web.PerformanceTimingNames.RESPONSE_END] diff --git a/packages/opentelemetry-tracing/src/Tracer.ts b/packages/opentelemetry-tracing/src/Tracer.ts index 0bdb41d2a7f..db19b66c8a7 100644 --- a/packages/opentelemetry-tracing/src/Tracer.ts +++ b/packages/opentelemetry-tracing/src/Tracer.ts @@ -177,15 +177,6 @@ function getParent( options: api.SpanOptions, context: api.Context ): api.SpanContext | undefined { - if (options.parent === null) return undefined; - if (options.parent) return getContext(options.parent); + if (options.root) return undefined; return api.getParentSpanContext(context); } - -function getContext(span: api.Span | api.SpanContext): api.SpanContext { - return isSpan(span) ? span.context() : span; -} - -function isSpan(span: api.Span | api.SpanContext): span is api.Span { - return typeof (span as api.Span).context === 'function'; -} diff --git a/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts b/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts index ac204f2770e..f378805bd5a 100644 --- a/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts +++ b/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts @@ -210,49 +210,13 @@ describe('BasicTracerProvider', () => { childSpan.end(); }); - it('should override context parent with option parent', () => { - const tracer = new BasicTracerProvider().getTracer('default'); - const span = tracer.startSpan('my-span'); - const overrideParent = tracer.startSpan('my-parent-override-span'); - const childSpan = tracer.startSpan( - 'child-span', - { - parent: overrideParent, - }, - setActiveSpan(ROOT_CONTEXT, span) - ); - const context = childSpan.context(); - assert.strictEqual(context.traceId, overrideParent.context().traceId); - assert.strictEqual(context.traceFlags, TraceFlags.SAMPLED); - span.end(); - childSpan.end(); - }); - - it('should override context parent with option parent context', () => { - const tracer = new BasicTracerProvider().getTracer('default'); - const span = tracer.startSpan('my-span'); - const overrideParent = tracer.startSpan('my-parent-override-span'); - const childSpan = tracer.startSpan( - 'child-span', - { - parent: overrideParent.context(), - }, - setActiveSpan(ROOT_CONTEXT, span) - ); - const context = childSpan.context(); - assert.strictEqual(context.traceId, overrideParent.context().traceId); - assert.strictEqual(context.traceFlags, TraceFlags.SAMPLED); - span.end(); - childSpan.end(); - }); - - it('should create a root span when parent is null', () => { + it('should create a root span when root is true', () => { const tracer = new BasicTracerProvider().getTracer('default'); const span = tracer.startSpan('my-span'); const overrideParent = tracer.startSpan('my-parent-override-span'); const rootSpan = tracer.startSpan( 'root-span', - { parent: null }, + { root: true }, setActiveSpan(ROOT_CONTEXT, span) ); const context = rootSpan.context(); From 6216b002c7178d259f85595fe34f150be10e17da Mon Sep 17 00:00:00 2001 From: Shivkanya Andhare <62445341+shivkanya9146@users.noreply.github.com> Date: Tue, 3 Nov 2020 02:12:36 +0530 Subject: [PATCH 4/5] chore(zipkin): export ExporterConfig (#1474) Co-authored-by: Daniel Dyla --- packages/opentelemetry-exporter-zipkin/src/index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/opentelemetry-exporter-zipkin/src/index.ts b/packages/opentelemetry-exporter-zipkin/src/index.ts index 4879fea44f9..cdf0ae281d6 100644 --- a/packages/opentelemetry-exporter-zipkin/src/index.ts +++ b/packages/opentelemetry-exporter-zipkin/src/index.ts @@ -14,5 +14,6 @@ * limitations under the License. */ -export * from './zipkin'; export * from './platform'; +export { ExporterConfig } from './types'; +export * from './zipkin'; From b1e2bedc73b2ccf523a7c3e21e75500d87d5c4e8 Mon Sep 17 00:00:00 2001 From: Shivkanya Andhare <62445341+shivkanya9146@users.noreply.github.com> Date: Tue, 3 Nov 2020 23:44:15 +0530 Subject: [PATCH 5/5] chore(jaeger): export ExporterConfig (#1478) Co-authored-by: Bartlomiej Obecny Co-authored-by: Daniel Dyla --- packages/opentelemetry-exporter-jaeger/src/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/opentelemetry-exporter-jaeger/src/index.ts b/packages/opentelemetry-exporter-jaeger/src/index.ts index ca7eafd1d71..520ddca099b 100644 --- a/packages/opentelemetry-exporter-jaeger/src/index.ts +++ b/packages/opentelemetry-exporter-jaeger/src/index.ts @@ -15,3 +15,4 @@ */ export * from './jaeger'; +export { ExporterConfig } from './types';