Skip to content
This repository has been archived by the owner on Nov 10, 2022. It is now read-only.

Commit

Permalink
feat: move debuglogging global to the end of the registration
Browse files Browse the repository at this point in the history
... that's to assure we really did register it, not only attempted.
Any short-circuits will be logged as well, so the debug would be
unneccessary in those cases.
  • Loading branch information
rauno56 committed May 19, 2021
1 parent 6a9aa81 commit 2d71eae
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 35 deletions.
8 changes: 4 additions & 4 deletions src/internal/global-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,6 @@ export function registerGlobal<Type extends keyof OTelGlobalAPI>(
instance: OTelGlobalAPI[Type],
allowOverride = false
): boolean {
diag.debug(
`@opentelemetry/api: Registering a global for ${type} v${VERSION}.`
);

const api = (_global[GLOBAL_OPENTELEMETRY_API_KEY] = _global[
GLOBAL_OPENTELEMETRY_API_KEY
] ?? {
Expand All @@ -64,6 +60,10 @@ export function registerGlobal<Type extends keyof OTelGlobalAPI>(
}

api[type] = instance;
diag.debug(
`@opentelemetry/api: Registered a global for ${type} v${VERSION}.`
);

return true;
}

Expand Down
70 changes: 39 additions & 31 deletions test/internal/global.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import * as assert from 'assert';
import { getGlobal } from '../../src/internal/global-utils';
import { _globalThis } from '../../src/platform';
import { NoopContextManager } from '../../src/context/NoopContextManager';
import { DiagLogLevel } from '../../src/diag/types';
import sinon = require('sinon');

const api1 = require('../../src') as typeof import('../../src');
Expand All @@ -32,6 +33,14 @@ const api2 = require('../../src') as typeof import('../../src');
// It is intentionally not autogenerated to ensure the author of the change is aware of what they are doing.
const GLOBAL_API_SYMBOL_KEY = 'io.opentelemetry.js.api.1';

const getMockLogger = () => ({
verbose: sinon.spy(),
debug: sinon.spy(),
info: sinon.spy(),
warn: sinon.spy(),
error: sinon.spy(),
});

describe('Global Utils', () => {
// prove they are separate instances
assert.notEqual(api1, api2);
Expand Down Expand Up @@ -91,48 +100,47 @@ describe('Global Utils', () => {
assert.strictEqual(api1.context['_getContextManager'](), original);
});

it('should debug log registrations', () => {
const logger = getMockLogger();
api1.diag.setLogger(logger, DiagLogLevel.DEBUG);

const newContextManager = new NoopContextManager();
api1.context.setGlobalContextManager(newContextManager);

sinon.assert.calledWith(logger.debug, sinon.match(/global for context/));
sinon.assert.calledWith(logger.debug, sinon.match(/global for diag/));
sinon.assert.calledTwice(logger.debug);
});

it('should log an error if there is a duplicate registration', () => {
const error = sinon.stub();
api1.diag.setLogger({
verbose: () => {},
debug: () => {},
info: () => {},
warn: () => {},
error,
});
const logger = getMockLogger();
api1.diag.setLogger(logger);

api1.context.setGlobalContextManager(new NoopContextManager());
api1.context.setGlobalContextManager(new NoopContextManager());

sinon.assert.calledOnce(error);
assert.strictEqual(error.firstCall.args.length, 1);
sinon.assert.calledOnce(logger.error);
assert.strictEqual(logger.error.firstCall.args.length, 1);
assert.ok(
error.firstCall.args[0].startsWith(
logger.error.firstCall.args[0].startsWith(
'Error: @opentelemetry/api: Attempted duplicate registration of API: context'
)
);
});

it('should allow duplicate registration of the diag logger', () => {
const error1 = sinon.stub();
const error2 = sinon.stub();
api1.diag.setLogger({
verbose: () => {},
debug: () => {},
info: () => {},
warn: () => {},
error: error1,
});

api1.diag.setLogger({
verbose: () => {},
debug: () => {},
info: () => {},
warn: () => {},
error: error2,
});

sinon.assert.notCalled(error1);
sinon.assert.notCalled(error2);
const logger1 = getMockLogger();
const logger2 = getMockLogger();

api1.diag.setLogger(logger1);
api1.diag.setLogger(logger2);

sinon.assert.notCalled(logger1.error);
sinon.assert.calledOnce(logger1.warn);
sinon.assert.calledWith(logger1.warn, sinon.match(/will be overwritten/i));

sinon.assert.notCalled(logger2.error);
sinon.assert.calledOnce(logger2.warn);
sinon.assert.calledWith(logger2.warn, sinon.match(/will overwrite/i));
});
});

0 comments on commit 2d71eae

Please sign in to comment.