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

Simplify SDK registration #837

Merged
merged 7 commits into from
Mar 10, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions packages/opentelemetry-api/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export * from './common/Logger';
export * from './common/Time';
export * from './context/propagation/carrier';
export * from './context/propagation/HttpTextFormat';
export * from './context/propagation/NoopHttpTextFormat';
export * from './distributed_context/DistributedContext';
export * from './distributed_context/EntryValue';
export * from './metrics/BoundInstrument';
Expand Down
1 change: 1 addition & 0 deletions packages/opentelemetry-node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
"devDependencies": {
"@types/mocha": "^5.2.5",
"@types/node": "^12.6.8",
"@opentelemetry/scope-base": "^0.4.0",
"@types/semver": "^6.0.1",
"@types/shimmer": "^1.0.1",
"codecov": "^3.6.1",
Expand Down
20 changes: 18 additions & 2 deletions packages/opentelemetry-node/src/NodeTracerProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,20 @@
* limitations under the License.
*/

import { BasicTracerProvider } from '@opentelemetry/tracing';
import { AsyncHooksScopeManager } from '@opentelemetry/scope-async-hooks';
import {
BasicTracerProvider,
SDKRegistrationConfig,
} from '@opentelemetry/tracing';
import { DEFAULT_INSTRUMENTATION_PLUGINS, NodeTracerConfig } from './config';
import { PluginLoader } from './instrumentation/PluginLoader';

/**
* This class represents a node tracer with `async_hooks` module.
* Register this TracerProvider for use with the OpenTelemetry API.
* Undefined values may be replaced with defaults, and
* null values will be skipped.
*
* @param config Configuration object for SDK registration
*/
export class NodeTracerProvider extends BasicTracerProvider {
private readonly _pluginLoader: PluginLoader;
Expand All @@ -37,4 +45,12 @@ export class NodeTracerProvider extends BasicTracerProvider {
stop() {
this._pluginLoader.unload();
}

register(config: SDKRegistrationConfig = {}) {
if (config.contextManager === undefined) {
dyladan marked this conversation as resolved.
Show resolved Hide resolved
config.contextManager = new AsyncHooksScopeManager();
dyladan marked this conversation as resolved.
Show resolved Hide resolved
}

super.register(config);
}
}
86 changes: 86 additions & 0 deletions packages/opentelemetry-node/test/registration.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/*!
* Copyright 2020, 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 {
context,
NoopHttpTextFormat,
NoopTracerProvider,
propagation,
trace,
} from '@opentelemetry/api';
import { HttpTraceContext } from '@opentelemetry/core';
import { AsyncHooksScopeManager } from '@opentelemetry/scope-async-hooks';
import { NoopScopeManager } from '@opentelemetry/scope-base';
import * as assert from 'assert';
import { NodeTracerProvider } from '../src';

describe('API registration', () => {
beforeEach(() => {
context.initGlobalContextManager(new NoopScopeManager());
propagation.initGlobalPropagator(new NoopHttpTextFormat());
trace.initGlobalTracerProvider(new NoopTracerProvider());
});

it('should register default implementations', () => {
const tracerProvider = new NodeTracerProvider();
tracerProvider.register();

assert.ok(context['_scopeManager'] instanceof AsyncHooksScopeManager);
assert.ok(propagation['_propagator'] instanceof HttpTraceContext);
assert.ok(trace['_tracerProvider'] === tracerProvider);
});

it('should register configured implementations', () => {
const tracerProvider = new NodeTracerProvider();

const contextManager = new NoopScopeManager();
const propagator = new NoopHttpTextFormat();

tracerProvider.register({
contextManager,
propagator,
});

assert.ok(context['_scopeManager'] === contextManager);
assert.ok(propagation['_propagator'] === propagator);

assert.ok(trace['_tracerProvider'] === tracerProvider);
});

it('should skip null context manager', () => {
const tracerProvider = new NodeTracerProvider();
tracerProvider.register({
contextManager: null,
});

assert.ok(context['_scopeManager'] instanceof NoopScopeManager);

assert.ok(propagation['_propagator'] instanceof HttpTraceContext);
assert.ok(trace['_tracerProvider'] === tracerProvider);
});

it('should skip null propagator', () => {
const tracerProvider = new NodeTracerProvider();
tracerProvider.register({
propagator: null,
});

assert.ok(propagation['_propagator'] instanceof NoopHttpTextFormat);

assert.ok(context['_scopeManager'] instanceof AsyncHooksScopeManager);
assert.ok(trace['_tracerProvider'] === tracerProvider);
});
});
32 changes: 27 additions & 5 deletions packages/opentelemetry-tracing/src/BasicTracerProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,23 @@
* limitations under the License.
*/

import { ConsoleLogger } from '@opentelemetry/core';
import * as types from '@opentelemetry/api';
import * as api from '@opentelemetry/api';
import { ConsoleLogger, HttpTraceContext } from '@opentelemetry/core';
import { SpanProcessor, Tracer } from '.';
import { DEFAULT_CONFIG } from './config';
import { MultiSpanProcessor } from './MultiSpanProcessor';
import { NoopSpanProcessor } from './NoopSpanProcessor';
import { TracerConfig } from './types';
import { SDKRegistrationConfig, TracerConfig } from './types';

/**
* This class represents a basic tracer provider which platform libraries can extend
*/
export class BasicTracerProvider implements types.TracerProvider {
export class BasicTracerProvider implements api.TracerProvider {
private readonly _registeredSpanProcessors: SpanProcessor[] = [];
private readonly _tracers: Map<string, Tracer> = new Map();

activeSpanProcessor = new NoopSpanProcessor();
readonly logger: types.Logger;
readonly logger: api.Logger;

constructor(private _config: TracerConfig = DEFAULT_CONFIG) {
this.logger = _config.logger || new ConsoleLogger(_config.logLevel);
Expand Down Expand Up @@ -59,4 +59,26 @@ export class BasicTracerProvider implements types.TracerProvider {
getActiveSpanProcessor(): SpanProcessor {
return this.activeSpanProcessor;
}

/**
* Register this TracerProvider for use with the OpenTelemetry API.
* Undefined values may be replaced with defaults, and
* null values will be skipped.
*
* @param config Configuration object for SDK registration
*/
register(config: SDKRegistrationConfig = {}) {
api.trace.initGlobalTracerProvider(this);
if (config.propagator === undefined) {
dyladan marked this conversation as resolved.
Show resolved Hide resolved
config.propagator = new HttpTraceContext();
}

if (config.contextManager) {
dyladan marked this conversation as resolved.
Show resolved Hide resolved
api.context.initGlobalContextManager(config.contextManager);
}

if (config.propagator) {
api.propagation.initGlobalPropagator(config.propagator);
}
}
}
21 changes: 20 additions & 1 deletion packages/opentelemetry-tracing/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,14 @@
* limitations under the License.
*/

import { Attributes, Logger, Sampler } from '@opentelemetry/api';
import {
Attributes,
HttpTextFormat,
Logger,
Sampler,
} from '@opentelemetry/api';
import { LogLevel } from '@opentelemetry/core';
import { ScopeManager } from '@opentelemetry/scope-base';

/**
* TracerConfig provides an interface for configuring a Basic Tracer.
Expand Down Expand Up @@ -44,6 +50,19 @@ export interface TracerConfig {
traceParams?: TraceParams;
}

/**
* Configuration options for registering the API with the SDK.
* Undefined values may be substituted for defaults, and null
* values will not be registered.
*/
export interface SDKRegistrationConfig {
/** Propagator to register as the global propagator */
propagator?: HttpTextFormat | null;
obecny marked this conversation as resolved.
Show resolved Hide resolved

/** Context manager to register as the global context manager */
contextManager?: ScopeManager | null;
dyladan marked this conversation as resolved.
Show resolved Hide resolved
}

/** Global configuration of trace service */
export interface TraceParams {
/** numberOfAttributesPerSpan is number of attributes per span */
Expand Down
22 changes: 21 additions & 1 deletion packages/opentelemetry-web/src/WebTracerProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@
*/

import { BasePlugin } from '@opentelemetry/core';
import { BasicTracerProvider, TracerConfig } from '@opentelemetry/tracing';
import { ZoneScopeManager } from '@opentelemetry/scope-zone';
Copy link
Member Author

Choose a reason for hiding this comment

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

Switch to stack

Copy link
Member

Choose a reason for hiding this comment

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

Is there any specific reason to use the stack scope manager by default ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Zone scope manager depends on zone.js which may not be available. If we have it as a dependency, then we may have a different zone.js than the end user. If we have it as a peer dependency, the scope manager would be broken for any user that doesn't have zone.js.

import {
BasicTracerProvider,
SDKRegistrationConfig,
TracerConfig,
} from '@opentelemetry/tracing';

/**
* WebTracerConfig provides an interface for configuring a Web Tracer.
Expand Down Expand Up @@ -45,4 +50,19 @@ export class WebTracerProvider extends BasicTracerProvider {
plugin.enable([], this, this.logger);
}
}

/**
* Register this TracerProvider for use with the OpenTelemetry API.
* Undefined values may be replaced with defaults, and
* null values will be skipped.
*
* @param config Configuration object for SDK registration
*/
register(config: SDKRegistrationConfig = {}) {
if (config.contextManager === undefined) {
dyladan marked this conversation as resolved.
Show resolved Hide resolved
config.contextManager = new ZoneScopeManager();
dyladan marked this conversation as resolved.
Show resolved Hide resolved
}

super.register(config);
}
}
86 changes: 86 additions & 0 deletions packages/opentelemetry-web/test/registration.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/*!
* Copyright 2020, 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 {
context,
NoopHttpTextFormat,
NoopTracerProvider,
propagation,
trace,
} from '@opentelemetry/api';
import { HttpTraceContext } from '@opentelemetry/core';
import { ZoneScopeManager } from '@opentelemetry/scope-zone';
import { NoopScopeManager } from '@opentelemetry/scope-base';
import * as assert from 'assert';
import { WebTracerProvider } from '../src';

describe('API registration', () => {
beforeEach(() => {
context.initGlobalContextManager(new NoopScopeManager());
propagation.initGlobalPropagator(new NoopHttpTextFormat());
trace.initGlobalTracerProvider(new NoopTracerProvider());
});

it('should register default implementations', () => {
const tracerProvider = new WebTracerProvider();
tracerProvider.register();

assert.ok(context['_scopeManager'] instanceof ZoneScopeManager);
assert.ok(propagation['_propagator'] instanceof HttpTraceContext);
assert.ok(trace['_tracerProvider'] === tracerProvider);
});

it('should register configured implementations', () => {
const tracerProvider = new WebTracerProvider();

const contextManager = new NoopScopeManager();
const propagator = new NoopHttpTextFormat();

tracerProvider.register({
contextManager,
propagator,
});

assert.ok(context['_scopeManager'] === contextManager);
assert.ok(propagation['_propagator'] === propagator);

assert.ok(trace['_tracerProvider'] === tracerProvider);
});

it('should skip null context manager', () => {
const tracerProvider = new WebTracerProvider();
tracerProvider.register({
contextManager: null,
});

assert.ok(context['_scopeManager'] instanceof NoopScopeManager);

assert.ok(propagation['_propagator'] instanceof HttpTraceContext);
assert.ok(trace['_tracerProvider'] === tracerProvider);
});

it('should skip null propagator', () => {
const tracerProvider = new WebTracerProvider();
tracerProvider.register({
propagator: null,
});

assert.ok(propagation['_propagator'] instanceof NoopHttpTextFormat);

assert.ok(context['_scopeManager'] instanceof ZoneScopeManager);
assert.ok(trace['_tracerProvider'] === tracerProvider);
});
});