Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
95 changes: 45 additions & 50 deletions experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import {
context,
propagation,
ProxyTracerProvider,
trace,
diag,
DiagLogLevel,
Expand Down Expand Up @@ -67,11 +66,10 @@ import {
defaultResource,
} from '@opentelemetry/resources';
import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-http';
import { logs, ProxyLoggerProvider } from '@opentelemetry/api-logs';
import { logs } from '@opentelemetry/api-logs';
import {
SimpleLogRecordProcessor,
InMemoryLogRecordExporter,
LoggerProvider,
ConsoleLogRecordExporter,
BatchLogRecordProcessor,
} from '@opentelemetry/sdk-logs';
Expand Down Expand Up @@ -100,8 +98,8 @@ function assertDefaultPropagatorRegistered() {
}

describe('Node SDK', () => {
let delegate: any;
let logsDelegate: any;
let setGlobalTracerProviderSpy: Sinon.SinonSpy;
let setGlobalLoggerProviderSpy: Sinon.SinonSpy;

beforeEach(() => {
diag.disable();
Expand All @@ -111,10 +109,8 @@ describe('Node SDK', () => {
metrics.disable();
logs.disable();

delegate = (trace.getTracerProvider() as ProxyTracerProvider).getDelegate();
logsDelegate = (
logs.getLoggerProvider() as ProxyLoggerProvider
)._getDelegate();
setGlobalTracerProviderSpy = Sinon.spy(trace, 'setGlobalTracerProvider');
setGlobalLoggerProviderSpy = Sinon.spy(logs, 'setGlobalLoggerProvider');
});

afterEach(() => {
Expand Down Expand Up @@ -145,15 +141,13 @@ describe('Node SDK', () => {
assertDefaultContextManagerRegistered();
assertDefaultPropagatorRegistered();

assert.strictEqual(
(trace.getTracerProvider() as ProxyTracerProvider).getDelegate(),
delegate,
assert.ok(
setGlobalTracerProviderSpy.called === false,
'tracer provider should not have changed'
);
Comment on lines +144 to 147
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: we could probably use

Sinon.assert.notCalled(setGlobalTracerProviderSpy);

for this type of assertion.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I kept using assert so we keep the error message but I agree maybe it's not necessary.

assert.ok(!(metrics.getMeterProvider() instanceof MeterProvider));
assert.strictEqual(
(logs.getLoggerProvider() as ProxyLoggerProvider)._getDelegate(),
logsDelegate,
assert.ok(
setGlobalLoggerProviderSpy.called === false,
'logger provider should not have changed'
);

Expand Down Expand Up @@ -204,9 +198,11 @@ describe('Node SDK', () => {
assertDefaultContextManagerRegistered();
assertDefaultPropagatorRegistered();

const apiTracerProvider =
trace.getTracerProvider() as ProxyTracerProvider;
assert.ok(apiTracerProvider.getDelegate() instanceof NodeTracerProvider);
assert.strictEqual(setGlobalTracerProviderSpy.callCount, 1);
assert.ok(
setGlobalTracerProviderSpy.lastCall.args[0] instanceof
NodeTracerProvider
);
Comment on lines +201 to +205
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: we could probably use

Sinon.assert.calledOnceWithMatch(
        setGlobalTracerProviderSpy,
        (arg: any) => {
          return arg instanceof NodeTracerProvider;
        }
      );

for this type of assertion.

await sdk.shutdown();
});

Expand All @@ -221,9 +217,15 @@ describe('Node SDK', () => {
assertDefaultContextManagerRegistered();
assertDefaultPropagatorRegistered();

const apiTracerProvider =
trace.getTracerProvider() as ProxyTracerProvider;
assert.ok(apiTracerProvider.getDelegate() instanceof NodeTracerProvider);
assert.strictEqual(
setGlobalTracerProviderSpy.callCount,
1,
'tracer provider should have changed once'
);
assert.ok(
setGlobalTracerProviderSpy.lastCall.args[0] instanceof
NodeTracerProvider
);
await sdk.shutdown();
});

Expand All @@ -244,10 +246,8 @@ describe('Node SDK', () => {
assertDefaultContextManagerRegistered();
assertDefaultPropagatorRegistered();

const apiTracerProvider =
trace.getTracerProvider() as ProxyTracerProvider;
const nodeTracerProvider = apiTracerProvider.getDelegate();

const nodeTracerProvider = setGlobalTracerProviderSpy.lastCall.args[0];
assert.strictEqual(setGlobalTracerProviderSpy.callCount, 1);
assert.ok(nodeTracerProvider instanceof NodeTracerProvider);

const spanProcessor = nodeTracerProvider['_activeSpanProcessor'] as any;
Expand Down Expand Up @@ -292,9 +292,8 @@ describe('Node SDK', () => {
assertDefaultContextManagerRegistered();
assertDefaultPropagatorRegistered();

assert.strictEqual(
(trace.getTracerProvider() as ProxyTracerProvider).getDelegate(),
delegate,
assert.ok(
setGlobalTracerProviderSpy.called === false,
'tracer provider should not have changed'
);

Expand Down Expand Up @@ -332,9 +331,8 @@ describe('Node SDK', () => {
assertDefaultContextManagerRegistered();
assertDefaultPropagatorRegistered();

assert.strictEqual(
(trace.getTracerProvider() as ProxyTracerProvider).getDelegate(),
delegate,
assert.ok(
setGlobalTracerProviderSpy.called === false,
'tracer provider should not have changed'
);

Expand Down Expand Up @@ -423,9 +421,8 @@ describe('Node SDK', () => {
assertDefaultContextManagerRegistered();
assertDefaultPropagatorRegistered();

assert.strictEqual(
(trace.getTracerProvider() as ProxyTracerProvider).getDelegate(),
delegate,
assert.ok(
setGlobalTracerProviderSpy.called === false,
'tracer provider should not have changed'
);

Expand All @@ -450,15 +447,14 @@ describe('Node SDK', () => {
assertDefaultContextManagerRegistered();
assertDefaultPropagatorRegistered();

assert.strictEqual(
(trace.getTracerProvider() as ProxyTracerProvider).getDelegate(),
delegate,
assert.ok(
setGlobalTracerProviderSpy.called === false,
'tracer provider should not have changed'
);

assert.ok(
(logs.getLoggerProvider() as ProxyLoggerProvider) instanceof
LoggerProvider
setGlobalLoggerProviderSpy.called === true,
'logger provider should have changed'
);
await sdk.shutdown();
});
Expand Down Expand Up @@ -637,9 +633,8 @@ describe('Node SDK', () => {
assertDefaultContextManagerRegistered();
assertDefaultPropagatorRegistered();

assert.strictEqual(
(trace.getTracerProvider() as ProxyTracerProvider).getDelegate(),
delegate,
assert.ok(
setGlobalTracerProviderSpy.called === false,
'tracer provider should not have changed'
);

Expand Down Expand Up @@ -1018,9 +1013,8 @@ describe('Node SDK', () => {
const sdk = new NodeSDK({});
sdk.start();

assert.strictEqual(
(trace.getTracerProvider() as ProxyTracerProvider).getDelegate(),
delegate,
assert.ok(
setGlobalTracerProviderSpy.called === false,
'sdk.start() should not change the global tracer provider'
);

Expand Down Expand Up @@ -1125,7 +1119,6 @@ describe('Node SDK', () => {
});

it('should not register the provider if OTEL_LOGS_EXPORTER contains none', async () => {
const logsAPIStub = Sinon.spy(logs, 'setGlobalLoggerProvider');
process.env.OTEL_LOGS_EXPORTER = 'console,none';
const sdk = new NodeSDK();
sdk.start();
Expand All @@ -1134,7 +1127,10 @@ describe('Node SDK', () => {
'OTEL_LOGS_EXPORTER contains "none". Logger provider will not be initialized.'
);

Sinon.assert.notCalled(logsAPIStub);
assert.ok(
setGlobalLoggerProviderSpy.callCount === 0,
'logger provider should not have changed'
);
await sdk.shutdown();
});

Expand Down Expand Up @@ -1715,9 +1711,8 @@ describe('Node SDK', () => {
'OTEL_TRACES_EXPORTER contains "none". SDK will not be initialized.'
);

assert.strictEqual(
(trace.getTracerProvider() as ProxyTracerProvider).getDelegate(),
delegate,
assert.ok(
setGlobalTracerProviderSpy.called === false,
'tracer provider should not have changed'
);

Expand Down
32 changes: 17 additions & 15 deletions packages/opentelemetry-sdk-trace-node/test/registration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,10 @@
*/

import * as assert from 'assert';
import * as sinon from 'sinon';
import { inspect } from 'util';

import {
context,
propagation,
trace,
ProxyTracerProvider,
} from '@opentelemetry/api';
import { context, propagation, trace } from '@opentelemetry/api';
import { AsyncLocalStorageContextManager } from '@opentelemetry/context-async-hooks';
import { CompositePropagator } from '@opentelemetry/core';
import { NodeTracerProvider } from '../src';
Expand All @@ -38,10 +34,17 @@ const assertInstanceOf = (actual: object, ExpectedConstructor: Function) => {
};

describe('API registration', function () {
let setGlobalTracerProviderSpy: sinon.SinonSpy;

beforeEach(() => {
context.disable();
trace.disable();
propagation.disable();
setGlobalTracerProviderSpy = sinon.spy(trace, 'setGlobalTracerProvider');
});

afterEach(() => {
sinon.restore();
});

it('should register default implementations', function () {
Expand All @@ -56,9 +59,8 @@ describe('API registration', function () {
propagation['_getGlobalPropagator'](),
CompositePropagator
);
const apiTracerProvider = trace.getTracerProvider() as ProxyTracerProvider;

assert.ok(apiTracerProvider.getDelegate() === tracerProvider);
assert.strictEqual(setGlobalTracerProviderSpy.callCount, 1);
assert.ok(setGlobalTracerProviderSpy.lastCall.args[0] === tracerProvider);
});

it('should register configured implementations', function () {
Expand All @@ -78,8 +80,8 @@ describe('API registration', function () {
assert.strictEqual(context['_getContextManager'](), mockContextManager);
assert.strictEqual(propagation['_getGlobalPropagator'](), mockPropagator);

const apiTracerProvider = trace.getTracerProvider() as ProxyTracerProvider;
assert.strictEqual(apiTracerProvider.getDelegate(), tracerProvider);
assert.strictEqual(setGlobalTracerProviderSpy.callCount, 1);
assert.ok(setGlobalTracerProviderSpy.lastCall.args[0] === tracerProvider);
});

it('should skip null context manager', function () {
Expand All @@ -100,8 +102,8 @@ describe('API registration', function () {
CompositePropagator
);

const apiTracerProvider = trace.getTracerProvider() as ProxyTracerProvider;
assert.ok(apiTracerProvider.getDelegate() === tracerProvider);
assert.strictEqual(setGlobalTracerProviderSpy.callCount, 1);
assert.ok(setGlobalTracerProviderSpy.lastCall.args[0] === tracerProvider);
});

it('should skip null propagator', function () {
Expand All @@ -119,7 +121,7 @@ describe('API registration', function () {
AsyncLocalStorageContextManager
);

const apiTracerProvider = trace.getTracerProvider() as ProxyTracerProvider;
assert.ok(apiTracerProvider.getDelegate() === tracerProvider);
assert.strictEqual(setGlobalTracerProviderSpy.callCount, 1);
assert.ok(setGlobalTracerProviderSpy.lastCall.args[0] === tracerProvider);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,16 @@
* limitations under the License.
*/

import {
context,
propagation,
trace,
ProxyTracerProvider,
} from '@opentelemetry/api';
import { context, propagation, trace } from '@opentelemetry/api';
import * as assert from 'assert';
import * as sinon from 'sinon';
import { StackContextManager, WebTracerProvider } from '../src';
import { resourceFromAttributes } from '@opentelemetry/resources';

describe('Node Globals Foolproofing', function () {
const originalProcess = globalThis?.process;
let setGlobalTracerProviderSpy: sinon.SinonSpy;

before(() => {
Object.assign(globalThis, { process: false });
});
Expand All @@ -38,6 +36,11 @@ describe('Node Globals Foolproofing', function () {
context.disable();
trace.disable();
propagation.disable();
setGlobalTracerProviderSpy = sinon.spy(trace, 'setGlobalTracerProvider');
});

afterEach(() => {
sinon.restore();
});

it('Can get TraceProvider without node globals such as process', function () {
Expand All @@ -54,8 +57,8 @@ describe('Node Globals Foolproofing', function () {
);

assert.ok(context['_getContextManager']() instanceof StackContextManager);
const apiTracerProvider = trace.getTracerProvider() as ProxyTracerProvider;
assert.ok(apiTracerProvider.getDelegate() === tracerProvider);
assert.strictEqual(setGlobalTracerProviderSpy.callCount, 1);
assert.ok(setGlobalTracerProviderSpy.lastCall.args[0] === tracerProvider);
});

it('Can get TraceProvider with custom id generator and without node globals such as process', function () {
Expand Down
Loading