adding event dispatcher #708
Conversation
per spec, "SDK implementations MUST allow end users to change the library's default error handling behavior for relevant errors"
Codecov Report
@@ Coverage Diff @@
## main #708 +/- ##
============================================
+ Coverage 82.41% 82.82% +0.41%
- Complexity 1247 1284 +37
============================================
Files 139 150 +11
Lines 3048 3127 +79
============================================
+ Hits 2512 2590 +78
- Misses 536 537 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| */ | ||
|
|
||
| //logger used by default event handlers | ||
| LoggerHolder::set( |
There was a problem hiding this comment.
@tidal - this might be an opportunity to move LoggerHolder somewhere under SDK\Event ? A way to get rid of the static singleton could be to create something like an AbstractLoggableEvent (or trait) which accepts a LoggerInterface and is extended by our events as required?
The idea of events came up in a SIG meeting a while back. Aside from being a way to log, the idea was that having events internally would be useful when it came to things like intra-signal messaging (eg, fire a metrics event on batch export with queue size and #records exported). I believe it can also help with #298. That said, since PSR-14 doesn't provide a mechanism to register event listeners, it concerns me a little that if we did have our own internal events (eg for metrics), then that functionality would be turned off by default if a user decides to use another PSR-14 package (and the onus is on them to register some/all events to re-enable that functionality). Would it then be safer to keep logging directly to a PSR-3 logger, and not expose our internal eventing? |
otel spec says that _all_ otel libraries (including API) should be able to expose troubleshooting telemetry, so enable that by moving events (and logging) into API
|
Discussed in SIG to shrink this PR back to just providing an event dispatcher. Error logging to be via psr-3 as we currently do. |
|
Further notes for the event system: ensure it stays public, and look at using CloudEvents as the basis for the events themselves. |
|
Just a quick conflict to resolve with the |
|
@tidal mentioned we want to tie this to an event system as well - cloud events |
To implement the otel spec requirement that
SDK implementations MUST allow end users to change the library's default error handling behavior for relevant errors, implement psr-14 compliant event system. It wraps our existing logging mechanism, so by default we still log errors etc as before, but it is now possible to provide any psr-14 implementation and handle events in some other way.In passing, fix up some
coversannotations in SpanTest which exposed some uncovered code inSpanLimitsFixes #299
Replaces #603