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

chore: remove tracer apis not part of spec #1764

Merged
merged 13 commits into from
Dec 21, 2020
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,16 @@ To request automatic tracing support for a module not on this list, please [file
|----------------------------------------------------------|-----------------------------------------------------------------------------------------|
| [@opentelemetry/shim-opentracing][otel-shim-opentracing] | OpenTracing shim allows existing OpenTracing instrumentation to report to OpenTelemetry |

## Upgrade guidelines

### 0.14.0 to 0.15.0
Flarna marked this conversation as resolved.
Show resolved Hide resolved

[PR-1764](https://github.com/open-telemetry/opentelemetry-js/pull/1764) removed some APIs from `Tracer`:

- `Tracer.getCurrentSpan()`: use `api.getActiveSpan(api.context.active()))`
Flarna marked this conversation as resolved.
Show resolved Hide resolved
Flarna marked this conversation as resolved.
Show resolved Hide resolved
- `Tracer.withSpan(span)`: use `api.context.with(api.setActiveSpan(api.context.active(), span))`
- `Tracer.bind(target)`: use `api.context.bind(target)`

## Useful links

- For more information on OpenTelemetry, visit: <https://opentelemetry.io/>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const axios = require("axios");
const { HttpTraceContext } = require("@opentelemetry/core");
const { BasicTracerProvider } = require("@opentelemetry/tracing");
const { context, propagation, trace, ROOT_CONTEXT } = require("@opentelemetry/api");
const { context, propagation, setActiveSpan, trace, ROOT_CONTEXT } = require("@opentelemetry/api");
const {
AsyncHooksContextManager,
} = require("@opentelemetry/context-async-hooks");
Expand Down Expand Up @@ -36,7 +36,7 @@ app.post("/verify-tracecontext", (req, res) => {
req.body.map((action) => {
const span = tracer.startSpan("propagate-w3c");
let promise;
tracer.withSpan(span, () => {
context.with(setActiveSpan(context.active(), span), () => {
const headers = {};
propagation.inject(context.active(), headers);
promise = axios
Expand Down
15 changes: 0 additions & 15 deletions packages/opentelemetry-api/src/trace/NoopTracer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ import { getParentSpanContext } from '../context/context';
* No-op implementations of {@link Tracer}.
*/
export class NoopTracer implements Tracer {
getCurrentSpan(): Span {
return NOOP_SPAN;
}

// startSpan starts a noop span.
startSpan(name: string, options?: SpanOptions, context?: Context): Span {
const root = Boolean(options?.root);
Expand All @@ -46,17 +42,6 @@ export class NoopTracer implements Tracer {
return NOOP_SPAN;
}
}

withSpan<T extends (...args: unknown[]) => ReturnType<T>>(
span: Span,
fn: T
): ReturnType<T> {
return fn();
}

bind<T>(target: T, _span?: Span): T {
return target;
}
}

function isSpanContext(spanContext: any): spanContext is SpanContext {
Expand Down
15 changes: 0 additions & 15 deletions packages/opentelemetry-api/src/trace/ProxyTracer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,25 +31,10 @@ export class ProxyTracer implements Tracer {
public readonly version?: string
) {}

getCurrentSpan(): Span | undefined {
return this._getTracer().getCurrentSpan();
}

startSpan(name: string, options?: SpanOptions): Span {
return this._getTracer().startSpan(name, options);
}

withSpan<T extends (...args: unknown[]) => ReturnType<T>>(
span: Span,
fn: T
): ReturnType<T> {
return this._getTracer().withSpan(span, fn);
}

bind<T>(target: T, span?: Span): T {
return this._getTracer().bind(target, span);
}

/**
* Try to get a tracer from the proxy tracer provider.
* If the proxy tracer provider has no delegate, return a noop tracer.
Expand Down
47 changes: 2 additions & 45 deletions packages/opentelemetry-api/src/trace/tracer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,24 +27,9 @@ import { SpanOptions } from './SpanOptions';
*/
export interface Tracer {
/**
* Returns the current Span from the current context if available.
* Starts a new {@link Span}. Start the span without setting it on context.
*
* If there is no Span associated with the current context, `undefined` is
* returned.
*
* To install a {@link Span} to the current Context use
* {@link Tracer.withSpan}.
*
* @returns Span The currently active Span
*/
getCurrentSpan(): Span | undefined;

/**
* Starts a new {@link Span}. Start the span without setting it as the current
* span in this tracer's context.
*
* This method do NOT modify the current Context. To install a {@link
* Span} to the current Context use {@link Tracer.withSpan}.
* This method do NOT modify the current Context.
*
* @param name The name of the span
* @param [options] SpanOptions used for span creation
Expand All @@ -56,32 +41,4 @@ export interface Tracer {
* span.end();
*/
startSpan(name: string, options?: SpanOptions, context?: Context): Span;

/**
* Executes the function given by fn within the context provided by Span.
*
* This is a convenience method for creating spans attached to the tracer's
* context. Applications that need more control over the span lifetime should
* use {@link Tracer.startSpan} instead.
*
* @param span The span that provides the context
* @param fn The function to be executed inside the provided context
* @example
* tracer.withSpan(span, () => {
* tracer.getCurrentSpan().addEvent("parent's event");
* doSomeOtherWork(); // Here "span" is the current Span.
* });
*/
withSpan<T extends (...args: unknown[]) => ReturnType<T>>(
span: Span,
fn: T
): ReturnType<T>;

/**
* Bind a span as the target's context or propagate the current one.
*
* @param target Any object to which a context need to be set
* @param [context] Optionally specify the context which you want to bind
*/
bind<T>(target: T, context?: Span): T;
}
16 changes: 0 additions & 16 deletions packages/opentelemetry-api/test/api/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ import api, {
} from '../../src';

describe('API', () => {
const functions = ['getCurrentSpan', 'startSpan', 'withSpan'];

it('should expose a tracer provider via getTracerProvider', () => {
const tracer = api.trace.getTracerProvider();
assert.ok(tracer);
Expand All @@ -59,20 +57,6 @@ describe('API', () => {
metrics.disable();
});

it('should not crash', () => {
functions.forEach(fn => {
const tracer = api.trace.getTracerProvider();
try {
((tracer as unknown) as { [fn: string]: Function })[fn](); // Try to run the function
assert.ok(true, fn);
} catch (err) {
if (err.message !== 'Method not implemented.') {
assert.ok(true, fn);
}
}
});
});

it('should use the global tracer provider', () => {
api.trace.setGlobalTracerProvider(new TestTracerProvider());
const tracer = api.trace.getTracerProvider().getTracer('name');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,24 +40,6 @@ describe('NoopTracer', () => {
}),
NOOP_SPAN
);

assert.deepStrictEqual(tracer.getCurrentSpan(), NOOP_SPAN);
});

it('should not crash when .withSpan()', done => {
const tracer = new NoopTracer();
tracer.withSpan(NOOP_SPAN, () => {
return done();
});
});

it('should not crash when .bind()', done => {
const tracer = new NoopTracer();
const fn = () => {
return done();
};
const patchedFn = tracer.bind(fn, NOOP_SPAN);
return patchedFn();
});

it('should propagate valid spanContext on the span (from context)', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ describe('ProxyTracer', () => {
}),
NOOP_SPAN
);

assert.deepStrictEqual(tracer.getCurrentSpan(), NOOP_SPAN);
});
});

Expand Down Expand Up @@ -96,18 +94,9 @@ describe('ProxyTracer', () => {
beforeEach(() => {
delegateSpan = new NoopSpan();
delegateTracer = {
bind(target) {
return target;
},
getCurrentSpan() {
return delegateSpan;
},
startSpan() {
return delegateSpan;
},
withSpan(span, fn) {
return fn();
},
};

tracer = provider.getTracer('test');
Expand Down
5 changes: 3 additions & 2 deletions packages/opentelemetry-context-zone-peer-dep/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ npm install --save @opentelemetry/context-zone-peer-dep
## Usage

```js
import { context } from '@opentelemetry/api';
import { ConsoleSpanExporter, SimpleSpanProcessor } from '@opentelemetry/tracing';
import { WebTracerProvider } from '@opentelemetry/web';
import { ZoneContextManager } from '@opentelemetry/context-zone-peer-dep';
Expand All @@ -35,12 +36,12 @@ providerWithZone.register({
// Example how the ZoneContextManager keeps the reference to the correct context during async operations
const webTracerWithZone = providerWithZone.getTracer('default');
const span1 = webTracerWithZone.startSpan('foo1');
webTracerWithZone.withSpan(span1, () => {
context.with(setActiveSpan(context.active(), span1, () => {
console.log('Current span is span1', webTracerWithZone.getCurrentSpan() === span1);
setTimeout(() => {
const span2 = webTracerWithZone.startSpan('foo2');
console.log('Current span is span1', webTracerWithZone.getCurrentSpan() === span1);
webTracerWithZone.withSpan(span2, () => {
context.with(setActiveSpan(context.active(), span2, () => {
console.log('Current span is span2', webTracerWithZone.getCurrentSpan() === span2);
setTimeout(() => {
console.log('Current span is span2', webTracerWithZone.getCurrentSpan() === span2);
Expand Down
6 changes: 4 additions & 2 deletions packages/opentelemetry-context-zone/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ npm install --save @opentelemetry/context-zone
## Usage

```js
import { context } from '@opentelemetry/api';
import { ConsoleSpanExporter, SimpleSpanProcessor } from '@opentelemetry/tracing';
import { WebTracerProvider } from '@opentelemetry/web';
import { ZoneContextManager } from '@opentelemetry/context-zone';
Expand All @@ -32,12 +33,13 @@ provider.register({
// Example how the ZoneContextManager keeps the reference to the correct context during async operations
const webTracerWithZone = providerWithZone.getTracer('default');
const span1 = webTracerWithZone.startSpan('foo1');
webTracerWithZone.withSpan(span1, () => {

context.with(setActiveSpan(context.active(), span1), () => {
console.log('Current span is span1', webTracerWithZone.getCurrentSpan() === span1);
setTimeout(() => {
const span2 = webTracerWithZone.startSpan('foo2');
console.log('Current span is span1', webTracerWithZone.getCurrentSpan() === span1);
webTracerWithZone.withSpan(span2, () => {
context.with(setActiveSpan(context.active(), span2), () => {
console.log('Current span is span2', webTracerWithZone.getCurrentSpan() === span2);
setTimeout(() => {
console.log('Current span is span2', webTracerWithZone.getCurrentSpan() === span2);
Expand Down
10 changes: 6 additions & 4 deletions packages/opentelemetry-grpc-utils/test/grpcUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import {
NoopTracerProvider,
SpanKind,
propagation,
getActiveSpan,
setActiveSpan,
} from '@opentelemetry/api';
import {
NoopLogger,
Expand Down Expand Up @@ -527,8 +529,8 @@ export const runTests = (
const span = provider
.getTracer('default')
.startSpan('TestSpan', { kind: SpanKind.PRODUCER });
return provider.getTracer('default').withSpan(span, async () => {
const rootSpan = provider.getTracer('default').getCurrentSpan();
return context.with(setActiveSpan(context.active(), span), async () => {
const rootSpan = getActiveSpan(context.active());
if (!rootSpan) {
return assert.ok(false);
}
Expand Down Expand Up @@ -623,8 +625,8 @@ export const runTests = (
const span = provider
.getTracer('default')
.startSpan('TestSpan', { kind: SpanKind.PRODUCER });
return provider.getTracer('default').withSpan(span, async () => {
const rootSpan = provider.getTracer('default').getCurrentSpan();
return context.with(setActiveSpan(context.active(), span), async () => {
const rootSpan = getActiveSpan(context.active());
if (!rootSpan) {
return assert.ok(false);
}
Expand Down
9 changes: 5 additions & 4 deletions packages/opentelemetry-instrumentation-http/src/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
SpanContext,
TraceFlags,
ROOT_CONTEXT,
getActiveSpan,
} from '@opentelemetry/api';
import { NoRecordingSpan } from '@opentelemetry/core';
import type * as http from 'http';
Expand Down Expand Up @@ -304,7 +305,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
this._callResponseHook(span, response);
}

this.tracer.bind(response);
context.bind(response);
this._logger.debug('outgoingRequest on response()');
response.on('end', () => {
this._logger.debug('outgoingRequest on end()');
Expand Down Expand Up @@ -410,7 +411,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
spanOptions
);

return instrumentation.tracer.withSpan(span, () => {
return context.with(setActiveSpan(context.active(), span), () => {
context.bind(request);
context.bind(response);

Expand Down Expand Up @@ -559,7 +560,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
'%s instrumentation outgoingRequest',
component
);
instrumentation.tracer.bind(request);
context.bind(request);
return instrumentation._traceClientRequest(
component,
request,
Expand All @@ -580,7 +581,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
: this._getConfig().requireParentforIncomingSpans;

let span: Span;
const currentSpan = this.tracer.getCurrentSpan();
const currentSpan = getActiveSpan(context.active());

if (requireParent === true && currentSpan === undefined) {
// TODO: Refactor this when a solution is found in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ describe('HttpInstrumentation', () => {

beforeEach(() => {
NOOP_TRACER.startSpan = sinon.spy();
NOOP_TRACER.withSpan = sinon.spy();
});

afterEach(() => {
Expand All @@ -79,11 +78,6 @@ describe('HttpInstrumentation', () => {
(NOOP_TRACER.startSpan as sinon.SinonSpy).called,
false
);

assert.strictEqual(
(NOOP_TRACER.withSpan as sinon.SinonSpy).called,
false
);
});
});
});
Expand Down
Loading