opentelemetry-js
opentelemetry-js copied to clipboard
fix(api): make failed diag.setLogger not be silent; correct versions in incompat error message
This fixes two issues in registerGlobal handling in the API.
make failed diag.setLogger not be silent
First, in the following script the second diag.setLogger fails to register, because API v1.7.0 is incompatible with the already registered API v1.5.0. That is as expected. However, that failure is silent, even though a perfectly functional DiagConsoleLogger() is passed in.
// silent-diag-reg-fail.js
//
// Usage:
// cd opentelemetry-js/api/
// npm install --no-save api150@npm:@opentelemetry/[email protected]
// node silent-diag-reg-fail.js
const api170 = require('.'); // @opentelemetry/[email protected]
const api150 = require('api150'); // npm install --no-save api150@npm:@opentelemetry/[email protected]
api150.diag.setLogger(new api150.DiagConsoleLogger(), api150.DiagLogLevel.DEBUG);
api150.diag.info('hi 1.5.0');
// (1)
api170.diag.setLogger(new api170.DiagConsoleLogger(), api170.DiagLogLevel.DEBUG);
api170.diag.info('hi 1.7.0');
Running this yields:
% node silent-diag-reg-fail.js
@opentelemetry/api: Registered a global for diag v1.5.0.
hi 1.5.0
https://github.com/open-telemetry/opentelemetry-js/blob/f5ef8de1cc92ad22d7d95df6a5f585f9d64ddef1/api/src/internal/global-utils.ts#L33-L59
The registerGlobal(...) is given a diag argument that is used to log the failed register.
The issue is here:
https://github.com/open-telemetry/opentelemetry-js/blob/f5ef8de1cc92ad22d7d95df6a5f585f9d64ddef1/api/src/api/diag.ts#L104
When the DiagAPI registers the newLogger, it passes self as that diag argument. self here is an object which proxies .error(), .info(), et al methods through the registered diag global (via getGlobal('diag')). Because the registration fails, getGlobal('diag') returns undefined and the error message is lost.
However, if it instead passes in newLogger, then any registration failures will get logged:
% node silent-diag-reg-fail.js
@opentelemetry/api: Registered a global for diag v1.5.0.
hi 1.5.0
Error: @opentelemetry/api: Registration of version v1.5.0 for diag does not match previously registered API v1.7.0
at registerGlobal (/Users/trentm/tm/opentelemetry-js2/api/build/src/internal/global-utils.js:41:21)
at DiagAPI.setLogger (/Users/trentm/tm/opentelemetry-js2/api/build/src/api/diag.js:69:54)
at Object.<anonymous> (/Users/trentm/tm/opentelemetry-js2/api/silent-diag-reg-fail.js:15:13)
at Module._compile (node:internal/modules/cjs/loader:1256:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1310:10)
at Module.load (node:internal/modules/cjs/loader:1119:32)
at Module._load (node:internal/modules/cjs/loader:960:12)
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:86:12)
at node:internal/main/run_main_module:23:47
correct versions in incompat error message
Second, notice how that error message has the versions backwards:
Registration of version v1.5.0 for diag does not match previously registered API v1.7.0
The previously registered version was v1.5.0. This PR reverses those versions in the error message:
% node silent-diag-reg-fail.js
@opentelemetry/api: Registered a global for diag v1.5.0.
hi 1.5.0
Error: @opentelemetry/api: Registration of version v1.7.0 for diag does not match previously registered API v1.5.0
at registerGlobal (/Users/trentm/tm/opentelemetry-js2/api/build/src/internal/global-utils.js:38:21)
...
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 92.20%. Comparing base (
4daa264) to head (eec90ea). Report is 213 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #4288 +/- ##
==========================================
- Coverage 92.64% 92.20% -0.44%
==========================================
Files 307 332 +25
Lines 9019 9447 +428
Branches 1919 2003 +84
==========================================
+ Hits 8356 8711 +355
- Misses 663 736 +73
| Files | Coverage Δ | |
|---|---|---|
| api/src/api/diag.ts | 100.00% <100.00%> (ø) |
|
| api/src/internal/global-utils.ts | 100.00% <ø> (ø) |
This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.
This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.
This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.