Skip to content

Commit 8d0a0a7

Browse files
jeengbemaryliag
jeengbe
and
maryliag
authored
fix: default to otlp if OTEL_METRICS_EXPORTER is empty (#6092)
Co-authored-by: Marylia Gutierrez <[email protected]>
1 parent bc60b8d commit 8d0a0a7

File tree

4 files changed

+78
-33
lines changed

4 files changed

+78
-33
lines changed

‎experimental/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2
2020
### :bug: Bug Fixes
2121

2222
* fix(instrumentation-grpc): attach correct name to diag message [#6097](https://github.com/open-telemetry/opentelemetry-js/pull/6043) @pichlermarc
23+
* fix(opentelemetry-sdk-node): default to otlp if OTEL_METRICS_EXPORTER is empty [#6092](https://github.com/open-telemetry/opentelemetry-js/pull/6092) @jeengbe
2324

2425
### :books: Documentation
2526

‎experimental/packages/opentelemetry-sdk-node/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ This is an alternative to programmatically configuring an exporter or span proce
213213
| OTEL_EXPORTER_OTLP_TRACES_PROTOCOL | The transport protocol to use on OTLP trace requests. Options include `grpc`, `http/protobuf`, and `http/json`. Default is `http/protobuf`. |
214214
| OTEL_EXPORTER_OTLP_METRICS_PROTOCOL | The transport protocol to use on OTLP metric requests. Options include `grpc`, `http/protobuf`, and `http/json`. Default is `http/protobuf`. |
215215
| OTEL_EXPORTER_OTLP_LOGS_PROTOCOL | The transport protocol to use on OTLP log requests. Options include `grpc`, `http/protobuf`, and `http/json`. Default is `http/protobuf`. |
216-
| OTEL_METRICS_EXPORTER | Metrics exporter to be used. options are `otlp`, `prometheus`, `console` or `none`. |
216+
| OTEL_METRICS_EXPORTER | Metrics exporter to be used. options are `otlp`, `prometheus`, `console` or `none`. Default is `otlp`. |
217217
| OTEL_METRIC_EXPORT_INTERVAL | The export interval when using a push Metric Reader. Default is `60000`. |
218218
| OTEL_METRIC_EXPORT_TIMEOUT | The export timeout when using a push Metric Reader. Default is `30000`. |
219219

‎experimental/packages/opentelemetry-sdk-node/src/sdk.ts‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -113,13 +113,13 @@ function getValueInMillis(envName: string, defaultValue: number): number {
113113
*/
114114
function configureMetricProviderFromEnv(): IMetricReader[] {
115115
const metricReaders: IMetricReader[] = [];
116-
const enabledExporters = getStringListFromEnv('OTEL_METRICS_EXPORTER');
117-
if (!enabledExporters) {
118-
return metricReaders;
119-
}
116+
const enabledExporters = Array.from(
117+
new Set(getStringListFromEnv('OTEL_METRICS_EXPORTER') ?? [])
118+
);
120119

121120
if (enabledExporters.length === 0) {
122121
diag.debug('OTEL_METRICS_EXPORTER is empty. Using default otlp exporter.');
122+
enabledExporters.push('otlp');
123123
}
124124

125125
if (enabledExporters.includes('none')) {
@@ -132,8 +132,10 @@ function configureMetricProviderFromEnv(): IMetricReader[] {
132132
enabledExporters.forEach(exporter => {
133133
if (exporter === 'otlp') {
134134
const protocol =
135-
process.env.OTEL_EXPORTER_OTLP_METRICS_PROTOCOL?.trim() ||
136-
process.env.OTEL_EXPORTER_OTLP_PROTOCOL?.trim();
135+
(
136+
getStringFromEnv('OTEL_EXPORTER_OTLP_METRICS_PROTOCOL') ??
137+
getStringFromEnv('OTEL_EXPORTER_OTLP_PROTOCOL')
138+
)?.trim() || 'http/protobuf'; // Using || to also fall back on empty string
137139

138140
const exportIntervalMillis = getValueInMillis(
139141
'OTEL_METRIC_EXPORT_INTERVAL',
@@ -461,7 +463,9 @@ export class NodeSDK {
461463
}
462464

463465
private configureLoggerProviderFromEnv(): void {
464-
const enabledExporters = getStringListFromEnv('OTEL_LOGS_EXPORTER') ?? [];
466+
const enabledExporters = Array.from(
467+
new Set(getStringListFromEnv('OTEL_LOGS_EXPORTER') ?? [])
468+
);
465469

466470
if (enabledExporters.length === 0) {
467471
diag.debug('OTEL_LOGS_EXPORTER is empty. Using default otlp exporter.');
@@ -479,10 +483,11 @@ export class NodeSDK {
479483

480484
enabledExporters.forEach(exporter => {
481485
if (exporter === 'otlp') {
482-
const protocol = (
483-
getStringFromEnv('OTEL_EXPORTER_OTLP_LOGS_PROTOCOL') ??
484-
getStringFromEnv('OTEL_EXPORTER_OTLP_PROTOCOL')
485-
)?.trim();
486+
const protocol =
487+
(
488+
getStringFromEnv('OTEL_EXPORTER_OTLP_LOGS_PROTOCOL') ??
489+
getStringFromEnv('OTEL_EXPORTER_OTLP_PROTOCOL')
490+
)?.trim() || 'http/protobuf'; // Using || to also fall back on empty string
486491

487492
switch (protocol) {
488493
case 'grpc':
@@ -494,10 +499,6 @@ export class NodeSDK {
494499
case 'http/protobuf':
495500
exporters.push(new OTLPProtoLogExporter());
496501
break;
497-
case undefined:
498-
case '':
499-
exporters.push(new OTLPProtoLogExporter());
500-
break;
501502
default:
502503
diag.warn(
503504
`Unsupported OTLP logs protocol: "${protocol}". Using http/protobuf.`

‎experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts‎

Lines changed: 60 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,10 @@ describe('Node SDK', () => {
131131
});
132132

133133
it('should not register more than the minimal SDK components', async () => {
134-
// need to set these to none, since the deafult value is 'otlp'
134+
// need to set these to none, since the default value is 'otlp'
135135
process.env.OTEL_TRACES_EXPORTER = 'none';
136136
process.env.OTEL_LOGS_EXPORTER = 'none';
137-
process.env.OTEL_METRIC_EXPORTER = 'none';
137+
process.env.OTEL_METRICS_EXPORTER = 'none';
138138
const sdk = new NodeSDK({
139139
autoDetectResources: false,
140140
});
@@ -156,6 +156,7 @@ describe('Node SDK', () => {
156156
logsDelegate,
157157
'logger provider should not have changed'
158158
);
159+
159160
await sdk.shutdown();
160161
});
161162

@@ -200,8 +201,6 @@ describe('Node SDK', () => {
200201

201202
sdk.start();
202203

203-
assert.ok(!(metrics.getMeterProvider() instanceof MeterProvider));
204-
205204
assertDefaultContextManagerRegistered();
206205
assertDefaultPropagatorRegistered();
207206

@@ -219,8 +218,6 @@ describe('Node SDK', () => {
219218

220219
sdk.start();
221220

222-
assert.ok(!(metrics.getMeterProvider() instanceof MeterProvider));
223-
224221
assertDefaultContextManagerRegistered();
225222
assertDefaultPropagatorRegistered();
226223

@@ -244,8 +241,6 @@ describe('Node SDK', () => {
244241

245242
sdk.start();
246243

247-
assert.ok(!(metrics.getMeterProvider() instanceof MeterProvider));
248-
249244
assertDefaultContextManagerRegistered();
250245
assertDefaultPropagatorRegistered();
251246

@@ -1245,13 +1240,6 @@ describe('Node SDK', () => {
12451240
delete process.env.OTEL_EXPORTER_METRICS_PROTOCOL;
12461241
});
12471242

1248-
it('should not register the provider if OTEL_METRICS_EXPORTER is not set', async () => {
1249-
const sdk = new NodeSDK();
1250-
sdk.start();
1251-
assert.ok(!(metrics.getMeterProvider() instanceof MeterProvider));
1252-
await sdk.shutdown();
1253-
});
1254-
12551243
it('should not register the provider if OTEL_METRICS_EXPORTER contains none', async () => {
12561244
process.env.OTEL_METRICS_EXPORTER = 'console,none';
12571245
const sdk = new NodeSDK();
@@ -1441,7 +1429,7 @@ describe('Node SDK', () => {
14411429
await sdk.shutdown();
14421430
});
14431431

1444-
it('should use prometheus if that is set ', async () => {
1432+
it('should use prometheus if that is set', async () => {
14451433
process.env.OTEL_METRICS_EXPORTER = 'prometheus';
14461434
delete process.env.OTEL_EXPORTER_OTLP_METRICS_PROTOCOL;
14471435
const sdk = new NodeSDK();
@@ -1454,9 +1442,44 @@ describe('Node SDK', () => {
14541442
);
14551443
await sdk.shutdown();
14561444
});
1445+
1446+
it('should use default grpc otlp exporter when empty value is provided for exporter via env', async () => {
1447+
process.env.OTEL_METRICS_EXPORTER = '';
1448+
const sdk = new NodeSDK();
1449+
sdk.start();
1450+
1451+
const meterProvider = metrics.getMeterProvider();
1452+
const sharedState = (meterProvider as any)['_sharedState'];
1453+
assert.ok(
1454+
sharedState.metricCollectors[0]._metricReader._exporter instanceof
1455+
OTLPProtoMetricExporter
1456+
);
1457+
await sdk.shutdown();
1458+
});
1459+
1460+
it('should not register the same exporter twice', async () => {
1461+
process.env.OTEL_METRICS_EXPORTER = 'console,otlp,console';
1462+
const sdk = new NodeSDK();
1463+
sdk.start();
1464+
1465+
const meterProvider = metrics.getMeterProvider();
1466+
const sharedState = (meterProvider as any)['_sharedState'];
1467+
1468+
assert.ok(sharedState.metricCollectors.length === 2);
1469+
assert.ok(
1470+
sharedState.metricCollectors[0]._metricReader._exporter instanceof
1471+
ConsoleMetricExporter
1472+
);
1473+
assert.ok(
1474+
sharedState.metricCollectors[1]._metricReader._exporter instanceof
1475+
OTLPProtoMetricExporter
1476+
);
1477+
1478+
await sdk.shutdown();
1479+
});
14571480
});
14581481

1459-
describe('setup exporter from env', () => {
1482+
describe('setup trace exporter from env', () => {
14601483
let stubLoggerError: Sinon.SinonStub;
14611484

14621485
const getSdkSpanProcessors = (sdk: NodeSDK) => {
@@ -1474,6 +1497,7 @@ describe('Node SDK', () => {
14741497
beforeEach(() => {
14751498
stubLoggerError = Sinon.stub(diag, 'warn');
14761499
});
1500+
14771501
afterEach(() => {
14781502
delete process.env.OTEL_EXPORTER_OTLP_PROTOCOL;
14791503
delete process.env.OTEL_EXPORTER_OTLP_TRACES_PROTOCOL;
@@ -1751,5 +1775,24 @@ describe('Node SDK', () => {
17511775
);
17521776
await sdk.shutdown();
17531777
});
1778+
1779+
it('should not register the same exporter twice', async () => {
1780+
process.env.OTEL_TRACES_EXPORTER = 'console,otlp,console';
1781+
const sdk = new NodeSDK();
1782+
sdk.start();
1783+
1784+
const listOfProcessors = getSdkSpanProcessors(sdk);
1785+
1786+
assert.ok(listOfProcessors.length === 2);
1787+
assert.ok(listOfProcessors[0] instanceof SimpleSpanProcessor);
1788+
assert.ok(
1789+
listOfProcessors[0]['_exporter'] instanceof ConsoleSpanExporter
1790+
);
1791+
assert.ok(listOfProcessors[1] instanceof BatchSpanProcessor);
1792+
assert.ok(
1793+
listOfProcessors[1]['_exporter'] instanceof OTLPProtoTraceExporter
1794+
);
1795+
await sdk.shutdown();
1796+
});
17541797
});
17551798
});

0 commit comments

Comments
 (0)