Skip to content

Commit

Permalink
fix: enable async hooks manager in node tracer close #226 (#232)
Browse files Browse the repository at this point in the history 
* fix: enable async hooks manager in node tracer

* fix(node-tracer): set scope manager as optional
  • Loading branch information
@vmarchaud @mayurkale22
vmarchaud authored and mayurkale22 committed Sep 7, 2019
1 parent 634f577 commit 3404e4f
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 33 deletions.
51 changes: 46 additions & 5 deletions packages/opentelemetry-node-tracer/src/NodeTracer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,47 @@
* limitations under the License.
*/

import { BasicTracer, BasicTracerConfig } from '@opentelemetry/basic-tracer';
import { BasicTracer } from '@opentelemetry/basic-tracer';
import { AsyncHooksScopeManager } from '@opentelemetry/scope-async-hooks';
import { ScopeManager } from '@opentelemetry/scope-base';
import {
Attributes,
BinaryFormat,
HttpTextFormat,
Logger,
Sampler,
} from '@opentelemetry/types';

/**
* NodeTracerConfig provides an interface for configuring a Node Tracer.
*/
export interface NodeTracerConfig {
/**
* Binary formatter which can serialize/deserialize Spans.
*/
binaryFormat?: BinaryFormat;
/**
* Attributed that will be applied on every span created by Tracer.
* Useful to add infrastructure and environment information to your spans.
*/
defaultAttributes?: Attributes;
/**
* HTTP text formatter which can inject/extract Spans.
*/
httpTextFormat?: HttpTextFormat;
/**
* User provided logger.
*/
logger?: Logger;
/**
* Sampler determines if a span should be recorded or should be a NoopSpan.
*/
sampler?: Sampler;
/**
* Scope manager keeps context across in-process operations.
*/
scopeManager?: ScopeManager;
}

/**
* This class represents a node tracer with `async_hooks` module.
Expand All @@ -24,10 +63,12 @@ export class NodeTracer extends BasicTracer {
/**
* Constructs a new Tracer instance.
*/
constructor(config: BasicTracerConfig) {
super(
Object.assign({}, { scopeManager: new AsyncHooksScopeManager() }, config)
);
constructor(config: NodeTracerConfig) {
if (config.scopeManager === undefined) {
config.scopeManager = new AsyncHooksScopeManager();
config.scopeManager.enable();
}
super(Object.assign({}, { scopeManager: config.scopeManager }, config));

// @todo: Integrate Plugin Loader (pull/126).
}
Expand Down
54 changes: 26 additions & 28 deletions packages/opentelemetry-node-tracer/test/NodeTracer.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,21 @@ import { NodeTracer } from '../src/NodeTracer';
import { TraceOptions } from '@opentelemetry/types';
import { Span } from '@opentelemetry/basic-tracer';

const sleep = (time: number) =>
new Promise(resolve => {
return setTimeout(resolve, time);
});

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

it('should construct an instance with binary format', () => {
const tracer = new NodeTracer({
binaryFormat: new BinaryTraceContext(),
scopeManager: new AsyncHooksScopeManager(),
});
assert.ok(tracer instanceof NodeTracer);
});
Expand All @@ -56,14 +58,12 @@ describe('NodeTracer', () => {
it('should construct an instance with logger', () => {
const tracer = new NodeTracer({
logger: new NoopLogger(),
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,
});
assert.ok(tracer instanceof NodeTracer);
Expand All @@ -75,7 +75,6 @@ describe('NodeTracer', () => {
region: 'eu-west',
asg: 'my-asg',
},
scopeManager: new AsyncHooksScopeManager(),
});
assert.ok(tracer instanceof NodeTracer);
});
Expand All @@ -84,7 +83,6 @@ describe('NodeTracer', () => {
describe('.startSpan()', () => {
it('should start a span with name only', () => {
const tracer = new NodeTracer({
scopeManager: new AsyncHooksScopeManager(),
logger: new NoopLogger(),
});
const span = tracer.startSpan('my-span');
Expand All @@ -93,7 +91,6 @@ describe('NodeTracer', () => {

it('should start a span with name and options', () => {
const tracer = new NodeTracer({
scopeManager: new AsyncHooksScopeManager(),
logger: new NoopLogger(),
});
const span = tracer.startSpan('my-span', {});
Expand All @@ -103,7 +100,6 @@ describe('NodeTracer', () => {
it('should return a default span with no sampling', () => {
const tracer = new NodeTracer({
sampler: NEVER_SAMPLER,
scopeManager: new AsyncHooksScopeManager(),
logger: new NoopLogger(),
});
const span = tracer.startSpan('my-span');
Expand Down Expand Up @@ -132,18 +128,14 @@ describe('NodeTracer', () => {

describe('.getCurrentSpan()', () => {
it('should return null with AsyncHooksScopeManager when no span started', () => {
const tracer = new NodeTracer({
scopeManager: new AsyncHooksScopeManager(),
});
const tracer = new NodeTracer({});
assert.deepStrictEqual(tracer.getCurrentSpan(), null);
});
});

describe('.withSpan()', () => {
it('should run scope with AsyncHooksScopeManager scope manager', done => {
const tracer = new NodeTracer({
scopeManager: new AsyncHooksScopeManager(),
});
const tracer = new NodeTracer({});
const span = tracer.startSpan('my-span');
tracer.withSpan(span, () => {
assert.deepStrictEqual(tracer.getCurrentSpan(), span);
Expand All @@ -153,9 +145,7 @@ describe('NodeTracer', () => {
});

it('should run scope with AsyncHooksScopeManager scope manager with multiple spans', done => {
const tracer = new NodeTracer({
scopeManager: new AsyncHooksScopeManager(),
});
const tracer = new NodeTracer({});
const span = tracer.startSpan('my-span');
tracer.withSpan(span, () => {
assert.deepStrictEqual(tracer.getCurrentSpan(), span);
Expand All @@ -174,13 +164,25 @@ describe('NodeTracer', () => {
// @todo: below check is not running.
assert.deepStrictEqual(tracer.getCurrentSpan(), null);
});

it('should find correct scope with promises', done => {
const tracer = new NodeTracer({});
const span = tracer.startSpan('my-span');
tracer.withSpan(span, async () => {
for (let i = 0; i < 3; i++) {
await sleep(5).then(() => {
assert.deepStrictEqual(tracer.getCurrentSpan(), span);
});
}
return done();
});
assert.deepStrictEqual(tracer.getCurrentSpan(), null);
});
});

describe('.bind()', () => {
it('should bind scope with AsyncHooksScopeManager scope manager', done => {
const tracer = new NodeTracer({
scopeManager: new AsyncHooksScopeManager(),
});
const tracer = new NodeTracer({});
const span = tracer.startSpan('my-span');
const fn = () => {
assert.deepStrictEqual(tracer.getCurrentSpan(), span);
Expand All @@ -198,18 +200,14 @@ describe('NodeTracer', () => {

describe('.getBinaryFormat()', () => {
it('should get default binary formatter', () => {
const tracer = new NodeTracer({
scopeManager: new AsyncHooksScopeManager(),
});
const tracer = new NodeTracer({});
assert.ok(tracer.getBinaryFormat() instanceof BinaryTraceContext);
});
});

describe('.getHttpTextFormat()', () => {
it('should get default HTTP text formatter', () => {
const tracer = new NodeTracer({
scopeManager: new AsyncHooksScopeManager(),
});
const tracer = new NodeTracer({});
assert.ok(tracer.getHttpTextFormat() instanceof HttpTraceContext);
});
});
Expand Down

0 comments on commit 3404e4f

Please sign in to comment.