[x] This only affects the JavaScript OpenTelemetry library
Motivation
why-u-no-warn.js
const ioredis = require('ioredis'); // **Oops, required ioredis before instrumenting it.**
const {NodeSDK} = require('@opentelemetry/sdk-node');
const {OTLPTraceExporter} = require('@opentelemetry/exporter-trace-otlp-http');
const {IORedisInstrumentation} = require("@opentelemetry/instrumentation-ioredis");
const sdk = new NodeSDK({
traceExporter: new OTLPTraceExporter(),
instrumentations: [new IORedisInstrumentation()]
});
sdk.start();
async function main() {
// Do something with ioredis.
}
main();
This code has made an error by requireing the ioredis package before instrumenting it. That might mean that the instrumentation doesn't work. There is a diag.warn that is meant to warn about this case. However that diag.warn doesn't work with the above usage:
% OTEL_NODE_RESOURCE_DETECTORS=none OTEL_LOG_LEVEL=debug node why-u-no-warn.js
@opentelemetry/api: Registered a global for diag v1.9.0.
@opentelemetry/api: Registered a global for trace v1.9.0.
@opentelemetry/api: Registered a global for context v1.9.0.
@opentelemetry/api: Registered a global for propagation v1.9.0.
The reason it doesn't work is because the _warnOnPreloadedModules is called in <Instrumentation>.enable(), which is called in the Instrumentation constructor (via InstrumentationBase). This is before the NodeSDK has a chance to process the OTEL_LOG_LEVEL envvar and set a DiagConsoleLogger.
My understanding is that the normal course of operation is that registerInstrumentations() is eventually called to enable the instrumentations. I don't understand why .enable() is called in the InstrumentationBase constructor. (I also haven't yet dug into the history behind it.)
Question
Do you think we could get away from .enable()ing instrumentations in their constructor?
Motivation
why-u-no-warn.js
This code has made an error by
require
ing theioredis
package before instrumenting it. That might mean that the instrumentation doesn't work. There is adiag.warn
that is meant to warn about this case. However that diag.warn doesn't work with the above usage:The reason it doesn't work is because the
_warnOnPreloadedModules
is called in<Instrumentation>.enable()
, which is called in the Instrumentation constructor (viaInstrumentationBase
). This is before theNodeSDK
has a chance to process theOTEL_LOG_LEVEL
envvar and set aDiagConsoleLogger
.My understanding is that the normal course of operation is that
registerInstrumentations()
is eventually called to enable the instrumentations. I don't understand why.enable()
is called in the InstrumentationBase constructor. (I also haven't yet dug into the history behind it.)Question
Do you think we could get away from
.enable()
ing instrumentations in their constructor?