fix(nodejs): bundle missing winston-transport dependency in Lambda layer - #2517
Open
pujitha24 wants to merge 1 commit into
Open
fix(nodejs): bundle missing winston-transport dependency in Lambda layer#2517pujitha24 wants to merge 1 commit into
pujitha24 wants to merge 1 commit into
Conversation
Motivation: Enabling `winston` via OTEL_NODE_ENABLED_INSTRUMENTATIONS never exported winston logs to the configured OTLP backend. The layer's webpack build already emitted "Module not found: Error: Can't resolve '@opentelemetry/winston-transport'" for the require inside @opentelemetry/instrumentation-winston's patched `configure()`, which attaches an OpenTelemetryTransportV3 transport to export logs. Since @opentelemetry/winston-transport was declared as neither a dependency of @opentelemetry/instrumentation-winston nor of this layer's package.json, it was never bundled, the require always threw MODULE_NOT_FOUND inside a caught try/catch, and the transport (and therefore log export) was silently skipped. Log correlation (trace_id/span_id injection into log records via the patched write/log methods) is a separate code path and is unaffected by this bug. This does not confirm the race condition theorized in the report (async LoggerProvider creation racing synchronous instrumentation registration in wrapper.ts) - init.mjs awaits both wrapper.init() and wrapper.wrap() to completion, including LoggerProvider creation, before the Lambda handler module is ever loaded, so the LoggerProvider is already set by the time user code creates a winston logger. Approach: Add `@opentelemetry/winston-transport` to the layer's dependencies so it is bundled by webpack alongside the other auto-instrumentation packages. Add `winston` as a devDependency to exercise the real auto-instrumentation path in a new regression test. Validation: - `npm run compile:webpack` in nodejs/packages/layer: before this change, printed a "Module not found" warning for '@opentelemetry/winston-transport'; after, compiles cleanly with winston-transport bundled. - `npm run build` in nodejs/packages/layer: full build (webpack, externals install, packaging) succeeds and produces layer.zip. - Added nodejs/packages/layer/test/winston-instrumentation.spec.ts, which enables WinstonInstrumentation, requires winston, creates a logger, and asserts an OpenTelemetryTransportV3 transport gets attached. Verified this test fails with an assertion error when @opentelemetry/winston-transport is removed from node_modules (reproducing the reported symptom), and passes with the dependency present. - `npm test` in nodejs/packages/layer (test:cjs + test:esm): 16/16 passing, no regressions in existing wrapper/handler tests. - `npm run lint` in nodejs/packages/layer: clean. Report: open-telemetry#2065 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation:
Enabling
winstonvia OTEL_NODE_ENABLED_INSTRUMENTATIONS never exportedwinston logs to the configured OTLP backend. The layer's webpack build
already emitted "Module not found: Error: Can't resolve
'@opentelemetry/winston-transport'" for the require inside
@opentelemetry/instrumentation-winston's patched
configure(), whichattaches an OpenTelemetryTransportV3 transport to export logs. Since
@opentelemetry/winston-transport was declared as neither a dependency of
@opentelemetry/instrumentation-winston nor of this layer's package.json,
it was never bundled, the require always threw MODULE_NOT_FOUND inside a
caught try/catch, and the transport (and therefore log export) was
silently skipped. Log correlation (trace_id/span_id injection into log
records via the patched write/log methods) is a separate code path and
is unaffected by this bug.
This does not confirm the race condition theorized in the report (async
LoggerProvider creation racing synchronous instrumentation registration
in wrapper.ts) - init.mjs awaits both wrapper.init() and wrapper.wrap()
to completion, including LoggerProvider creation, before the Lambda
handler module is ever loaded, so the LoggerProvider is already set by
the time user code creates a winston logger.
Approach:
Add
@opentelemetry/winston-transportto the layer's dependencies so itis bundled by webpack alongside the other auto-instrumentation packages.
Add
winstonas a devDependency to exercise the real auto-instrumentationpath in a new regression test.
Validation:
npm run compile:webpackin nodejs/packages/layer: before this change,printed a "Module not found" warning for '@opentelemetry/winston-transport';
after, compiles cleanly with winston-transport bundled.
npm run buildin nodejs/packages/layer: full build (webpack, externalsinstall, packaging) succeeds and produces layer.zip.
enables WinstonInstrumentation, requires winston, creates a logger, and
asserts an OpenTelemetryTransportV3 transport gets attached. Verified this
test fails with an assertion error when @opentelemetry/winston-transport
is removed from node_modules (reproducing the reported symptom), and
passes with the dependency present.
npm testin nodejs/packages/layer (test:cjs + test:esm): 16/16 passing,no regressions in existing wrapper/handler tests.
npm run lintin nodejs/packages/layer: clean.Report: #2065
Signed-off-by: Pujitha Paladugu 10557236+pujitha24@users.noreply.github.com
Fixes #2065