Logging Handler #468
Logging Handler #468
Conversation
This is a very simple version of what is needed to support the logging spec and is in part duplicating code from the batch span processor. It has no overload protection and this will wait until we know what is planned for log sampling in the spec as it will overlap.
Codecov ReportBase: 73.61% // Head: 36.34% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #468 +/- ##
===========================================
- Coverage 73.61% 36.34% -37.28%
===========================================
Files 56 59 +3
Lines 1751 3555 +1804
===========================================
+ Hits 1289 1292 +3
- Misses 462 2263 +1801
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| resource=Resource, | ||
| config=Config, | ||
| batch=Batch}) when map_size(Batch) =/= 0 -> | ||
| _ = export(Exporter, Resource, Batch, Config), |
There was a problem hiding this comment.
I'm noticing that this call here does not let the exporter update its state. If it could, we'd also be able to drastically simplify the FSM by letting export/4 do the optional call to init_exporter/1 and return the new config as well as letting the exporter update its state on return.
I however don't know what the future plans are for this module, so this drastic simplification possibly clashes with said plans.
There was a problem hiding this comment.
Interesting idea... I planned to simplify it by breaking out the shared logic from the batch_processor module. If that works there too then it makes sense to do.
| ?LOG_INFO("OTLP grpc export failed with HTTP status code ~s", [Status]), | ||
| error; | ||
| {error, Reason} -> | ||
| ?LOG_INFO("OTLP grpc export failed with error: ~p", [Reason]), |
There was a problem hiding this comment.
is there any chance these logs messages are gonna be fed back into the OTel exporter and cause issues with write amplification?
There was a problem hiding this comment.
oh they definitely will
There was a problem hiding this comment.
Maybe a domain can be used here but then by default all these logs would be lost.. ugh
tsloughter
force-pushed
the
log-sdk
branch
3 times, most recently
from
e21f980 to
3c1205e
Compare
This is a very simple version of what is needed to support the logging spec and is in part duplicating code from the batch span processor.
It has no overload protection and this will wait until we know what is planned for log sampling in the spec as it will overlap.
Oh, and no tests. I've only manually tested by bringing up the collector and a rebar3 shell in which I add the handler and then see logs going to the collector through the collector's console output where it will log each log it receives.