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

feature(trace): adds named tracer factory #420

Closed
wants to merge 24 commits into from
Closed
Show file tree
Hide file tree
Changes from 19 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions examples/basic-tracer-node/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const opentelemetry = require('@opentelemetry/core');
const { BasicTracer, SimpleSpanProcessor } = require('@opentelemetry/tracing');
const { BasicTracerFactory, SimpleSpanProcessor } = require('@opentelemetry/tracing');
const { JaegerExporter } = require('@opentelemetry/exporter-jaeger');
const { ZipkinExporter } = require('@opentelemetry/exporter-zipkin');

Expand All @@ -17,14 +17,14 @@ if (EXPORTER.toLowerCase().startsWith('z')) {
exporter = new JaegerExporter(options);
}

const tracer = new BasicTracer();
// Initialize the OpenTelemetry APIs to use the BasicTracer bindings
opentelemetry.initGlobalTracerFactory(new BasicTracerFactory());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this use BasicTracerFactory.instance() instead of directly calling new?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this (initialization of global tracer factory and BasicTracerFactory) seems a little burdensome for the end users. I was thinking something like this:

// In case of BasicTracer
const opentelemetry = require('@opentelemetry/tracing');
opentelemetry.getTracerFactory().getTracer(); // should return the default BasicTracer.
opentelemetry.getTracerFactory().getTracer('mongodb'); // with name
opentelemetry.getTracerFactory().getTracer('redis', '1.0.0'); // with name and version
opentelemetry.getTracerFactory().addSpanProcessor(...);

// In case of NodeTracer
const opentelemetry = require('@opentelemetry/node');
opentelemetry.getTracerFactory().getTracer(); // should return the default NodeTracer.

// In case of WebTracer
const opentelemetry = require('@opentelemetry/web');
opentelemetry.getTracerFactory().getTracer(); // should return the default WebTracer.

The getTracerFactory function is responsible for returning the corresponding TracerFactory and we should create (and assign) the TracerFactory instance statically once you load the library (maybe in index.ts). WDYT?


const tracer = opentelemetry.getTracer();

// Configure span processor to send spans to the provided exporter
tracer.addSpanProcessor(new SimpleSpanProcessor(exporter));

// Initialize the OpenTelemetry APIs to use the BasicTracer bindings
opentelemetry.initGlobalTracer(tracer);

// Create a span. A span must be closed.
const span = opentelemetry.getTracer().startSpan('main');
for (let i = 0; i < 10; i++) {
Expand Down
9 changes: 5 additions & 4 deletions examples/grpc/setup.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
'use strict';

const opentelemetry = require('@opentelemetry/core');
const { NodeTracer } = require('@opentelemetry/node');
const { NodeTracerFactory } = require('@opentelemetry/node');
const { SimpleSpanProcessor } = require('@opentelemetry/tracing');
const { JaegerExporter } = require('@opentelemetry/exporter-jaeger');
const { ZipkinExporter } = require('@opentelemetry/exporter-zipkin');
const EXPORTER = process.env.EXPORTER || '';

function setupTracerAndExporters(service) {
const tracer = new NodeTracer({
const factory = new NodeTracerFactory({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instance?

plugins: {
grpc: {
enabled: true,
Expand All @@ -17,6 +17,7 @@ function setupTracerAndExporters(service) {
}
}
});
const tracer = factory.getTracer();

let exporter;
if (EXPORTER.toLowerCase().startsWith('z')) {
Expand All @@ -33,8 +34,8 @@ function setupTracerAndExporters(service) {

tracer.addSpanProcessor(new SimpleSpanProcessor(exporter));

// Initialize the OpenTelemetry APIs to use the BasicTracer bindings
opentelemetry.initGlobalTracer(tracer);
// Initialize the OpenTelemetry APIs to use the BasicTracerFactory bindings
opentelemetry.initGlobalTracerFactory(factory);
}

exports.setupTracerAndExporters = setupTracerAndExporters;
7 changes: 4 additions & 3 deletions examples/http/setup.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
'use strict';

const opentelemetry = require('@opentelemetry/core');
const { NodeTracer } = require('@opentelemetry/node');
const { NodeTracerFactory } = require('@opentelemetry/node');
const { SimpleSpanProcessor } = require('@opentelemetry/tracing');
const { JaegerExporter } = require('@opentelemetry/exporter-jaeger');
const { ZipkinExporter } = require('@opentelemetry/exporter-zipkin');
const EXPORTER = process.env.EXPORTER || '';

function setupTracerAndExporters(service) {
const tracer = new NodeTracer();
const factory = new NodeTracerFactory();
const tracer = factory.getTracer();

let exporter;
if (EXPORTER.toLowerCase().startsWith('z')) {
Expand All @@ -26,7 +27,7 @@ function setupTracerAndExporters(service) {
tracer.addSpanProcessor(new SimpleSpanProcessor(exporter));

// Initialize the OpenTelemetry APIs to use the BasicTracer bindings
opentelemetry.initGlobalTracer(tracer);
opentelemetry.initGlobalTracerFactory(factory);
}

exports.setupTracerAndExporters = setupTracerAndExporters;
8 changes: 5 additions & 3 deletions examples/https/setup.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
'use strict';

const opentelemetry = require('@opentelemetry/core');
const { NodeTracer } = require('@opentelemetry/node');
const { NodeTracerFactory } = require('@opentelemetry/node');
const { SimpleSpanProcessor } = require('@opentelemetry/tracing');
const { JaegerExporter } = require('@opentelemetry/exporter-jaeger');
const { ZipkinExporter } = require('@opentelemetry/exporter-zipkin');
const EXPORTER = process.env.EXPORTER || '';
process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0';

function setupTracerAndExporters(service) {
let exporter;
const tracer = new NodeTracer();
const factory = new NodeTracerFactory();
const tracer = factory.getTracer();

if (EXPORTER.toLowerCase().startsWith('z')) {
exporter = new ZipkinExporter({
Expand All @@ -26,7 +28,7 @@ function setupTracerAndExporters(service) {
tracer.addSpanProcessor(new SimpleSpanProcessor(exporter));

// Initialize the OpenTelemetry APIs to use the BasicTracer bindings
opentelemetry.initGlobalTracer(tracer);
opentelemetry.initGlobalTracerFactory(factory);
}

exports.setupTracerAndExporters = setupTracerAndExporters;
3 changes: 2 additions & 1 deletion packages/opentelemetry-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ export * from './trace/globaltracer-utils';
export * from './trace/instrumentation/BasePlugin';
export * from './trace/NoopSpan';
export * from './trace/NoopTracer';
export * from './trace/NoopTracerFactory';
export * from './trace/NoRecordingSpan';
export * from './trace/sampler/ProbabilitySampler';
export * from './trace/spancontext-utils';
export * from './trace/TracerDelegate';
export * from './trace/TracerFactoryDelegate';
export * from './trace/TraceState';
export * from './metrics/NoopMeter';
30 changes: 30 additions & 0 deletions packages/opentelemetry-core/src/trace/NoopTracerFactory.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*!
* Copyright 2019, OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as types from '@opentelemetry/types';
import { NoopTracer } from './NoopTracer';

export class NoopTracerFactory implements types.TracerFactory {
bg451 marked this conversation as resolved.
Show resolved Hide resolved
private readonly _tracer: types.Tracer;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not omit constructor entirely and do:

  private readonly _tracer = new NoopTracer();


constructor() {
this._tracer = new NoopTracer();
}

getTracer(name?: string, version?: string): types.Tracer {
return this._tracer;
}
}
101 changes: 0 additions & 101 deletions packages/opentelemetry-core/src/trace/TracerDelegate.ts

This file was deleted.

64 changes: 64 additions & 0 deletions packages/opentelemetry-core/src/trace/TracerFactoryDelegate.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*!
* Copyright 2019, OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as types from '@opentelemetry/types';
import { NoopTracerFactory } from './NoopTracerFactory';

// Acts a bridge to the global tracer factory that can be safely called before the
// global tracer factory is initialized. The purpose of the delegation is to avoid the
// sometimes nearly intractable initialization order problems that can arise in
// applications with a complex set of dependencies. Also allows for the factory
// to be changed/disabled during runtime without needing to change reference
// to the global factory.
export class TracerFactoryDelegate implements types.TracerFactory {
private _currentTracerFactory: types.TracerFactory;
private readonly _tracerFactory: types.TracerFactory | null;
private readonly _fallbackTracerFactory: types.TracerFactory;

// Wrap a TracerFactory with a TracerDelegateFactory. Provided factory becomes the default
// fallback factory for when a global factory has not been initialized
constructor(
tracerFactory?: types.TracerFactory,
fallbackTracerFactory?: types.TracerFactory
) {
this._tracerFactory = tracerFactory || null;
this._fallbackTracerFactory =
fallbackTracerFactory || new NoopTracerFactory();
this._currentTracerFactory =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we have here this.start(); instead?

this._tracerFactory || this._fallbackTracerFactory; // equivalent to this.start()
}

// Begin using the user provided tracer factory. Stop always falling back to fallback tracer
// factory.
start(): void {
this._currentTracerFactory =
this._tracerFactory || this._fallbackTracerFactory;
}

// Stop the delegate from using the provided tracer factory. Begin to use the fallback factory
stop(): void {
this._currentTracerFactory = this._fallbackTracerFactory;
}

// -- TracerFactory interface implementation below -- //
getTracer(name?: string, version?: string): types.Tracer {
return this._currentTracerFactory.getTracer.apply(
bg451 marked this conversation as resolved.
Show resolved Hide resolved
this._currentTracerFactory,
// tslint:disable-next-line:no-any
arguments as any
);
}
}
23 changes: 15 additions & 8 deletions packages/opentelemetry-core/src/trace/globaltracer-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,28 @@
*/

import * as types from '@opentelemetry/types';
import { TracerDelegate } from './TracerDelegate';
import { TracerFactoryDelegate } from './TracerFactoryDelegate';

let globalTracerDelegate = new TracerDelegate();
let globalTracerFactoryDelegate = new TracerFactoryDelegate();

/**
* Set the current global tracer. Returns the initialized global tracer
*/
export function initGlobalTracer(tracer: types.Tracer): types.Tracer {
return (globalTracerDelegate = new TracerDelegate(tracer));
export function initGlobalTracerFactory(
tracerFactory: types.TracerFactory
): types.TracerFactory {
return (globalTracerFactoryDelegate = new TracerFactoryDelegate(
tracerFactory
));
}

export function getTracerFactory(): types.TracerFactory {
return globalTracerFactoryDelegate;
}

/**
* Returns the global tracer
* Finds or creates tracer from the global TracerFactory
*/
export function getTracer(): types.Tracer {
// Return the global tracer delegate
return globalTracerDelegate;
export function getTracer(name?: string, version?: string): types.Tracer {
return globalTracerFactoryDelegate.getTracer(name, version);
}
27 changes: 27 additions & 0 deletions packages/opentelemetry-core/test/trace/NoopTracerFactory.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*!
* Copyright 2019, OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as assert from 'assert';
import { NoopTracerFactory } from '../../src/trace/NoopTracerFactory';
import { NOOP_SPAN } from '../../src/trace/NoopSpan';

describe('NoopTracerFactory', () => {
it('should return a NOOP_SPAN', () => {
const factory = new NoopTracerFactory();
const span = factory.getTracer().startSpan('my-span');
assert.deepStrictEqual(span, NOOP_SPAN);
});
});
Loading