Skip to content

Commit 1d74508

Browse files
committed
feat: prevent the setup from running twice
This ensures that we can't accidentally re-instrument the code or run the setup script multiple times, which would have some weird consequences. This also sets us up to add host metrics, which otherwise would have been difficult to manage.
1 parent ca2216e commit 1d74508

File tree

3 files changed

+131
-38
lines changed

3 files changed

+131
-38
lines changed

packages/opentelemetry/README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,8 @@ opentelemetry.setup({ /* ... */ });
152152
</tr>
153153
</table>
154154

155+
This method returns any SDK instances created during setup. Calling this method a second time will return the same instances without rerunning setup.
156+
155157
### Running in production
156158

157159
#### Production metrics

packages/opentelemetry/lib/index.js

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ const { createInstrumentationConfig } = require('./config/instrumentations');
22
const { createMetricsConfig } = require('./config/metrics');
33
const { createResourceConfig } = require('./config/resource');
44
const { createTracingConfig } = require('./config/tracing');
5-
const opentelemetrySDK = require('@opentelemetry/sdk-node');
5+
const opentelemetry = require('@opentelemetry/sdk-node');
66
const logger = require('@dotcom-reliability-kit/logger');
77

88
/**
@@ -23,17 +23,36 @@ const logger = require('@dotcom-reliability-kit/logger');
2323
* Configuration options for OpenTelemetry tracing.
2424
*/
2525

26+
/**
27+
* @typedef {object} Instances
28+
* @property {opentelemetry.NodeSDK} sdk
29+
* A singleton instance of the OpenTelemetry Node SDK.
30+
*/
31+
32+
/**
33+
* Stores the singleton instances that were created during OpenTelemetry setup.
34+
*
35+
* @type {Instances}
36+
*/
37+
let instances;
38+
2639
/**
2740
* Set up OpenTelemetry tracing.
2841
*
2942
* @param {Options} [options]
3043
* OpenTelemetry configuration options.
44+
* @returns {Instances}
45+
* Returns any created SDK instances.
3146
*/
3247
function setupOpenTelemetry({
3348
authorizationHeader,
3449
metrics: metricsOptions,
3550
tracing: tracingOptions
3651
} = {}) {
52+
if (instances) {
53+
return instances;
54+
}
55+
3756
// We don't support using the built-in `OTEL_`-prefixed environment variables. We
3857
// do want to know when these are used, though, so that we can easily spot when
3958
// an app's use of these environment variables might be interfering.
@@ -51,31 +70,35 @@ function setupOpenTelemetry({
5170
// the LOG_LEVEL environment variable) takes over. We set the
5271
// OpenTelemetry log level to the maximum value that we want
5372
// Reliability Kit to consider logging
54-
opentelemetrySDK.api.diag.setLogger(
73+
opentelemetry.api.diag.setLogger(
5574
// @ts-ignore this complains because DiagLogger accepts a type
5675
// of unknown whereas our logger is stricter. This is fine though,
5776
// if something unknown is logged then we do our best with it.
5877
// It's easier to ignore this error than fix it.
5978
logger.createChildLogger({ event: 'OTEL_INTERNALS' }),
60-
opentelemetrySDK.api.DiagLogLevel.INFO
79+
opentelemetry.api.DiagLogLevel.INFO
6180
);
6281

6382
// Set up and start OpenTelemetry
64-
const sdk = new opentelemetrySDK.NodeSDK({
65-
// Configurations we set regardless of whether we're using tracing
66-
instrumentations: createInstrumentationConfig(),
67-
resource: createResourceConfig(),
83+
instances = {
84+
sdk: new opentelemetry.NodeSDK({
85+
// Configurations we set regardless of whether we're using tracing
86+
instrumentations: createInstrumentationConfig(),
87+
resource: createResourceConfig(),
6888

69-
// Add metrics-specific configurations
70-
...createMetricsConfig(metricsOptions || {}),
89+
// Add metrics-specific configurations
90+
...createMetricsConfig(metricsOptions || {}),
7191

72-
// Add tracing-specific configurations
73-
...createTracingConfig({
74-
authorizationHeader,
75-
...tracingOptions
92+
// Add tracing-specific configurations
93+
...createTracingConfig({
94+
authorizationHeader,
95+
...tracingOptions
96+
})
7697
})
77-
});
78-
sdk.start();
98+
};
99+
instances.sdk.start();
100+
101+
return instances;
79102
}
80103

81104
exports.setup = setupOpenTelemetry;

packages/opentelemetry/test/unit/lib/index.spec.js

Lines changed: 91 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,26 +15,49 @@ jest.mock('../../../lib/config/tracing', () => ({
1515
createTracingConfig: jest.fn().mockReturnValue({ tracing: 'mock-tracing' })
1616
}));
1717

18-
const {
19-
createInstrumentationConfig
20-
} = require('../../../lib/config/instrumentations');
21-
const { createMetricsConfig } = require('../../../lib/config/metrics');
22-
const { createResourceConfig } = require('../../../lib/config/resource');
23-
const { createTracingConfig } = require('../../../lib/config/tracing');
24-
const { diag, DiagLogLevel } = require('@opentelemetry/sdk-node').api;
25-
const { NodeSDK } = require('@opentelemetry/sdk-node');
26-
const logger = require('@dotcom-reliability-kit/logger');
27-
28-
logger.createChildLogger.mockReturnValue('mock child logger');
29-
DiagLogLevel.INFO = 'mock info log level';
30-
31-
// Import the OTel function for testing
32-
const opentelemetry = require('../../../lib/index');
33-
3418
describe('@dotcom-reliability-kit/opentelemetry', () => {
19+
let createInstrumentationConfig;
20+
let createMetricsConfig;
21+
let createResourceConfig;
22+
let createTracingConfig;
23+
let diag;
24+
let DiagLogLevel;
25+
let logger;
26+
let NodeSDK;
27+
let opentelemetry;
28+
29+
// Helper function to reload all modules. We need this because the setup
30+
// method stores a global singleton so it's impossible to call it multiple
31+
// times with different configuration values normally. We need to do this
32+
// in the tests though
33+
function reloadAllModules() {
34+
jest.resetModules();
35+
createInstrumentationConfig =
36+
require('../../../lib/config/instrumentations').createInstrumentationConfig;
37+
createMetricsConfig =
38+
require('../../../lib/config/metrics').createMetricsConfig;
39+
createResourceConfig =
40+
require('../../../lib/config/resource').createResourceConfig;
41+
createTracingConfig =
42+
require('../../../lib/config/tracing').createTracingConfig;
43+
diag = require('@opentelemetry/sdk-node').api.diag;
44+
DiagLogLevel = require('@opentelemetry/sdk-node').api.DiagLogLevel;
45+
NodeSDK = require('@opentelemetry/sdk-node').NodeSDK;
46+
logger = require('@dotcom-reliability-kit/logger');
47+
48+
logger.createChildLogger.mockReturnValue('mock child logger');
49+
DiagLogLevel.INFO = 'mock info log level';
50+
51+
opentelemetry = require('../../../lib/index');
52+
}
53+
54+
beforeEach(reloadAllModules);
55+
3556
describe('.setup(options)', () => {
36-
beforeAll(() => {
37-
opentelemetry.setup({
57+
let instances;
58+
59+
beforeEach(() => {
60+
instances = opentelemetry.setup({
3861
tracing: {
3962
endpoint: 'mock-tracing-endpoint',
4063
samplePercentage: 137
@@ -93,9 +116,16 @@ describe('@dotcom-reliability-kit/opentelemetry', () => {
93116
expect(NodeSDK.prototype.start).toHaveBeenCalledTimes(1);
94117
});
95118

119+
it('returns the SDK instance', () => {
120+
expect(instances).toEqual({
121+
sdk: NodeSDK.mock.instances[0]
122+
});
123+
});
124+
96125
describe('when no options are set', () => {
97-
beforeAll(() => {
126+
beforeEach(() => {
98127
NodeSDK.mockClear();
128+
reloadAllModules();
99129
opentelemetry.setup();
100130
});
101131

@@ -111,8 +141,9 @@ describe('@dotcom-reliability-kit/opentelemetry', () => {
111141
});
112142

113143
describe('when an authorization header is passed into the root options (deprecated)', () => {
114-
beforeAll(() => {
144+
beforeEach(() => {
115145
createTracingConfig.mockReset();
146+
reloadAllModules();
116147
opentelemetry.setup({
117148
authorizationHeader: 'mock-authorization-header-root',
118149
tracing: {
@@ -131,8 +162,9 @@ describe('@dotcom-reliability-kit/opentelemetry', () => {
131162
});
132163

133164
describe('when an authorization header is passed into the tracing options', () => {
134-
beforeAll(() => {
165+
beforeEach(() => {
135166
createTracingConfig.mockReset();
167+
reloadAllModules();
136168
opentelemetry.setup({
137169
tracing: {
138170
authorizationHeader: 'mock-authorization-header-tracing',
@@ -151,8 +183,9 @@ describe('@dotcom-reliability-kit/opentelemetry', () => {
151183
});
152184

153185
describe('when an authorization header is passed into both the root options (deprecated) and tracing options', () => {
154-
beforeAll(() => {
186+
beforeEach(() => {
155187
createTracingConfig.mockReset();
188+
reloadAllModules();
156189
opentelemetry.setup({
157190
authorizationHeader: 'mock-authorization-header-root',
158191
tracing: {
@@ -172,8 +205,9 @@ describe('@dotcom-reliability-kit/opentelemetry', () => {
172205
});
173206

174207
describe('when OTEL_ environment variables are defined', () => {
175-
beforeAll(() => {
208+
beforeEach(() => {
176209
process.env.OTEL_MOCK = 'mock';
210+
reloadAllModules();
177211
opentelemetry.setup();
178212
});
179213

@@ -185,5 +219,39 @@ describe('@dotcom-reliability-kit/opentelemetry', () => {
185219
});
186220
});
187221
});
222+
223+
describe('when called a second time', () => {
224+
let returnValue1;
225+
let returnValue2;
226+
227+
beforeEach(() => {
228+
reloadAllModules();
229+
returnValue1 = opentelemetry.setup({
230+
tracing: {
231+
endpoint: 'mock-tracing-endpoint',
232+
samplePercentage: 137
233+
},
234+
metrics: {
235+
endpoint: 'mock-metrics-endpoint'
236+
}
237+
});
238+
returnValue2 = opentelemetry.setup();
239+
});
240+
241+
it('instantiates and starts the OpenTelemetry Node SDK once only', () => {
242+
expect(NodeSDK).toHaveBeenCalledTimes(1);
243+
expect(NodeSDK).toHaveBeenCalledWith({
244+
instrumentations: 'mock-instrumentations',
245+
resource: 'mock-resource',
246+
tracing: 'mock-tracing',
247+
metrics: 'mock-metrics'
248+
});
249+
expect(NodeSDK.prototype.start).toHaveBeenCalledTimes(1);
250+
});
251+
252+
it('returns the same instances on each call', () => {
253+
expect(returnValue1).toStrictEqual(returnValue2);
254+
});
255+
});
188256
});
189257
});

0 commit comments

Comments
 (0)