Closed TylerHelmuth closed 1 month ago
Attention: Patch coverage is 85.71429%
with 2 lines
in your changes are missing coverage. Please review.
Project coverage is 91.66%. Comparing base (
7b046d9
) to head (ae7ef0b
).
Files | Patch % | Lines |
---|---|---|
otelcol/collector.go | 85.71% | 1 Missing and 1 partial :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@TylerHelmuth What do you think about the approach here: https://github.com/open-telemetry/opentelemetry-collector/pull/9908?
I believe this would allow us to configure the logger by storing the logs until they can be printed by an instantiated logger.
Maybe thats worth it 🤷 I figured this is such a niche need that the extra complexity to pass the logs around wasn't worth it.
If being able to configure the logger that prints confmap logs is important, I can look into the technique used in https://github.com/open-telemetry/opentelemetry-collector/pull/9908, although I'd prefer not to expose the logs in the API.
Example of the "wait for the configured logger" approach: https://github.com/open-telemetry/opentelemetry-collector/pull/10008
Description
This PR creates a un-configurable logger to be used in
confmap.ProviderSettings
so that logging can occur while configuration is resolved. It configures the zap logger using reasonable default values since this logger will only be used while resolving config.Example of the logger in action
Link to tracking issue
Related to https://github.com/open-telemetry/opentelemetry-collector/issues/9162 Related to https://github.com/open-telemetry/opentelemetry-collector/issues/5615
Testing
If we like this approach I'll add tests