Skip to content

Commit

Permalink
fix: wip revert - gts fix issue on all packages
Browse files Browse the repository at this point in the history 
Signed-off-by: Olivier Albertini <[email protected]>
  • Loading branch information
@OlivierAlbertini
OlivierAlbertini committed Aug 5, 2019
1 parent d87e34e commit 0b98274
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 62 deletions.
27 changes: 9 additions & 18 deletions packages/opentelemetry-basic-tracer/src/BasicTracer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,7 @@
*/

import * as types from '@opentelemetry/types';
import {
ALWAYS_SAMPLER,
BinaryTraceContext,
HttpTraceContext,
NOOP_SPAN,
} from '@opentelemetry/core';
import { ALWAYS_SAMPLER, BinaryTraceContext, HttpTraceContext, NOOP_SPAN } from '@opentelemetry/core';
import { BinaryFormat, HttpTextFormat } from '@opentelemetry/types';
import { BasicTracerConfig } from '../src/types';
import { ScopeManager } from '@opentelemetry/scope-base';
Expand All @@ -34,7 +29,8 @@ export class BasicTracer implements types.Tracer {
private readonly _binaryFormat: types.BinaryFormat;
private readonly _httpTextFormat: types.HttpTextFormat;
private readonly _sampler: types.Sampler;
readonly scopeManager: ScopeManager;

protected readonly _scopeManager: ScopeManager;

/**
* Constructs a new Tracer instance.
Expand All @@ -44,7 +40,7 @@ export class BasicTracer implements types.Tracer {
this._defaultAttributes = config.defaultAttributes || {};
this._httpTextFormat = config.httpTextFormat || new HttpTraceContext();
this._sampler = config.sampler || ALWAYS_SAMPLER;
this.scopeManager = config.scopeManager;
this._scopeManager = config.scopeManager;
}

/**
Expand All @@ -61,7 +57,7 @@ export class BasicTracer implements types.Tracer {
}

const spanOptions = Object.assign({}, options, {
parent: parentSpanContext,
parent: parentSpanContext
});
const span = new Span(this, name, spanOptions);

Expand All @@ -75,18 +71,15 @@ export class BasicTracer implements types.Tracer {
*/
getCurrentSpan(): types.Span {
// Get the current Span from the context.
return this.scopeManager.active() as types.Span;
return this._scopeManager.active() as types.Span;
}

/**
* Enters the scope of code where the given Span is in the current context.
*/
withSpan<T extends (...args: unknown[]) => ReturnType<T>>(
span: types.Span,
fn: T
): ReturnType<T> {
withSpan<T extends (...args: unknown[]) => ReturnType<T>>(span: types.Span, fn: T): ReturnType<T> {
// Set given span to context.
return this.scopeManager.with(span, fn);
return this._scopeManager.with(span, fn);
}

/**
Expand All @@ -110,9 +103,7 @@ export class BasicTracer implements types.Tracer {
return this._httpTextFormat;
}

private _getParentSpanContext(
parent: types.Span | types.SpanContext | undefined
): types.SpanContext | undefined {
private _getParentSpanContext(parent: types.Span | types.SpanContext | undefined): types.SpanContext | undefined {
if (!parent) return undefined;

// parent is a SpanContext
Expand Down
17 changes: 14 additions & 3 deletions packages/opentelemetry-node-tracer/src/NodeTracer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,21 @@ export class NodeTracer extends BasicTracer {
* Constructs a new Tracer instance.
*/
constructor(config: BasicTracerConfig) {
super(
Object.assign({}, { scopeManager: new AsyncHooksScopeManager() }, config)
);
super(Object.assign({}, { scopeManager: new AsyncHooksScopeManager() }, config));

// @todo: Integrate Plugin Loader (pull/126).
}
/**
* Binds the trace context to the given event emitter.
* This is necessary in order to create child spans correctly in event
* handlers.
* @param emitter An event emitter whose handlers should have
* the trace context binded to them.
*/
wrapEmitter(emitter: NodeJS.EventEmitter): void {
if (!this._scopeManager.active()) {
return;
}
this._scopeManager.bind(emitter);
}
}
74 changes: 54 additions & 20 deletions packages/opentelemetry-node-tracer/test/NodeTracer.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,48 +21,49 @@ import {
HttpTraceContext,
NEVER_SAMPLER,
NoopLogger,
NOOP_SPAN,
NOOP_SPAN
} from '@opentelemetry/core';
import { AsyncHooksScopeManager } from '@opentelemetry/scope-async-hooks';
import { NodeTracer } from '../src/NodeTracer';
import { EventEmitter } from 'events';

describe('NodeTracer', () => {
describe('constructor', () => {
it('should construct an instance with required only options', () => {
const tracer = new NodeTracer({
scopeManager: new AsyncHooksScopeManager(),
scopeManager: new AsyncHooksScopeManager()
});
assert.ok(tracer instanceof NodeTracer);
});

it('should construct an instance with binary format', () => {
const tracer = new NodeTracer({
binaryFormat: new BinaryTraceContext(),
scopeManager: new AsyncHooksScopeManager(),
scopeManager: new AsyncHooksScopeManager()
});
assert.ok(tracer instanceof NodeTracer);
});

it('should construct an instance with http text format', () => {
const tracer = new NodeTracer({
httpTextFormat: new HttpTraceContext(),
scopeManager: new AsyncHooksScopeManager(),
scopeManager: new AsyncHooksScopeManager()
});
assert.ok(tracer instanceof NodeTracer);
});

it('should construct an instance with logger', () => {
const tracer = new NodeTracer({
logger: new NoopLogger(),
scopeManager: new AsyncHooksScopeManager(),
scopeManager: new AsyncHooksScopeManager()
});
assert.ok(tracer instanceof NodeTracer);
});

it('should construct an instance with sampler', () => {
const tracer = new NodeTracer({
scopeManager: new AsyncHooksScopeManager(),
sampler: ALWAYS_SAMPLER,
sampler: ALWAYS_SAMPLER
});
assert.ok(tracer instanceof NodeTracer);
});
Expand All @@ -71,9 +72,9 @@ describe('NodeTracer', () => {
const tracer = new NodeTracer({
defaultAttributes: {
region: 'eu-west',
asg: 'my-asg',
asg: 'my-asg'
},
scopeManager: new AsyncHooksScopeManager(),
scopeManager: new AsyncHooksScopeManager()
});
assert.ok(tracer instanceof NodeTracer);
});
Expand All @@ -82,15 +83,15 @@ describe('NodeTracer', () => {
describe('.startSpan()', () => {
it('should start a span with name only', () => {
const tracer = new NodeTracer({
scopeManager: new AsyncHooksScopeManager(),
scopeManager: new AsyncHooksScopeManager()
});
const span = tracer.startSpan('my-span');
assert.ok(span);
});

it('should start a span with name and options', () => {
const tracer = new NodeTracer({
scopeManager: new AsyncHooksScopeManager(),
scopeManager: new AsyncHooksScopeManager()
});
const span = tracer.startSpan('my-span', {});
assert.ok(span);
Expand All @@ -99,7 +100,7 @@ describe('NodeTracer', () => {
it('should return a default span with no sampling', () => {
const tracer = new NodeTracer({
sampler: NEVER_SAMPLER,
scopeManager: new AsyncHooksScopeManager(),
scopeManager: new AsyncHooksScopeManager()
});
const span = tracer.startSpan('my-span');
assert.deepStrictEqual(span, NOOP_SPAN);
Expand All @@ -115,7 +116,7 @@ describe('NodeTracer', () => {
describe('.getCurrentSpan()', () => {
it('should return null with AsyncHooksScopeManager when no span started', () => {
const tracer = new NodeTracer({
scopeManager: new AsyncHooksScopeManager(),
scopeManager: new AsyncHooksScopeManager()
});
assert.deepStrictEqual(tracer.getCurrentSpan(), null);
});
Expand All @@ -124,7 +125,7 @@ describe('NodeTracer', () => {
describe('.withSpan()', () => {
it('should run scope with AsyncHooksScopeManager scope manager', done => {
const tracer = new NodeTracer({
scopeManager: new AsyncHooksScopeManager(),
scopeManager: new AsyncHooksScopeManager()
});
const span = tracer.startSpan('my-span');
tracer.withSpan(span, () => {
Expand All @@ -136,7 +137,7 @@ describe('NodeTracer', () => {

it('should run scope with AsyncHooksScopeManager scope manager with multiple spans', done => {
const tracer = new NodeTracer({
scopeManager: new AsyncHooksScopeManager(),
scopeManager: new AsyncHooksScopeManager()
});
const span = tracer.startSpan('my-span');
tracer.withSpan(span, () => {
Expand All @@ -145,10 +146,7 @@ describe('NodeTracer', () => {
const span1 = tracer.startSpan('my-span1', { parent: span });
tracer.withSpan(span1, () => {
assert.deepStrictEqual(tracer.getCurrentSpan(), span1);
assert.deepStrictEqual(
span1.context().traceId,
span.context().traceId
);
assert.deepStrictEqual(span1.context().traceId, span.context().traceId);
return done();
});
});
Expand All @@ -166,7 +164,7 @@ describe('NodeTracer', () => {
describe('getBinaryFormat', () => {
it('should get default binary formatter', () => {
const tracer = new NodeTracer({
scopeManager: new AsyncHooksScopeManager(),
scopeManager: new AsyncHooksScopeManager()
});
assert.ok(tracer.getBinaryFormat() instanceof BinaryTraceContext);
});
Expand All @@ -175,9 +173,45 @@ describe('NodeTracer', () => {
describe('.getHttpTextFormat()', () => {
it('should get default HTTP text formatter', () => {
const tracer = new NodeTracer({
scopeManager: new AsyncHooksScopeManager(),
scopeManager: new AsyncHooksScopeManager()
});
assert.ok(tracer.getHttpTextFormat() instanceof HttpTraceContext);
});
});
describe('.wrapEmitter()', () => {
it('should not throw', () => {
const tracer = new NodeTracer({
scopeManager: new AsyncHooksScopeManager()
});
tracer.wrapEmitter({} as EventEmitter);
});
// TODO: uncomment once https://github.com/open-telemetry/opentelemetry-js/pull/146 is merged
// it('should get current Span', (done) => {
// const tracer = new NodeTracer({
// scopeManager: new AsyncHooksScopeManager(),
// });

// const span = tracer.startSpan('my-span');
// class FakeEventEmitter extends EventEmitter {
// constructor() {
// super()
// }
// test() {
// this.emit('event');
// }
// }

// tracer.withSpan(span, () => {
// const fake = new FakeEventEmitter();
// tracer.wrapEmitter(fake);
// fake.on('event', () => {
// setTimeout(() => {
// assert.deepStrictEqual(tracer.getCurrentSpan(), span);
// done();
// }, 100);
// });
// fake.test();
// });
// });
});
});
Loading

0 comments on commit 0b98274

Please sign in to comment.