Skip to content

Commit

Permalink
test(instrumentation-pino): refactor Pino tests for correctness (#2247)
Browse files Browse the repository at this point in the history
Before this change the 'PinoInstrumentation' was being created
one or more times per test case, which results in the pino
module getting wrapped *each* time. The pino instrumentation
wraps the top-level/default export, so it cannot use the typical
instrumentation._wrap along with isWrapped/unwrap handling.
The result was a stack of pino() being wrapped over a dozen
times, with each of those 'patchedPino's being active.

This caused debugging complexity. For example, every test
case after the logKeys test case would get span context
fields being added as trace_id/span_id *and* traceId/spandId
because the earlier PinoInstrumentation was still active.
I suspect the 'pino-disabled.test.ts' tests were only
working because they ran *first*, because 'disabled' luckily
comes before 'enabled' alphabetically.

This refactors the tests back to a single file
(because mocha runs separate test files in the same
process there is some cross-talk between the files), and
works with a single instance of PinoInstrumentation.
FWIW the instr-bunyan and instr-winston tests work
similarly.
  • Loading branch information
trentm authored Jun 10, 2024
1 parent 93e7aab commit dadf308
Show file tree
Hide file tree
Showing 6 changed files with 432 additions and 520 deletions.
2 changes: 0 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
},
"devDependencies": {
"@opentelemetry/api": "^1.3.0",
"@opentelemetry/context-async-hooks": "^1.8.0",
"@opentelemetry/contrib-test-utils": "^0.40.0",
"@opentelemetry/sdk-trace-base": "^1.8.0",
"@opentelemetry/sdk-trace-node": "^1.8.0",
Expand Down
121 changes: 0 additions & 121 deletions plugins/node/opentelemetry-instrumentation-pino/test/common.ts

This file was deleted.

This file was deleted.

Loading

0 comments on commit dadf308

Please sign in to comment.