userver-framework / userver

Production-ready C++ Asynchronous Framework with rich functionality
https://userver.tech
Apache License 2.0
2.37k stars 275 forks source link

feat logging: Separate otlp config for logs and traces #676

Closed TertiumOrganum1 closed 1 week ago

TertiumOrganum1 commented 3 weeks ago

Currently, OTLP tracing and logging in userver are configured together, requiring the OTLP logger to be specified in the config file for both to be sent to the OTEL collector via the push model.

While this approach is suitable for tracing, logs should be output to STD-OUT or STD-ERR to allow the collector to gather them via the pull model. This approach is ideomatic fir Kubernetes, and is necessary because in case of service crash, logs output to STD-OUT won't be lost (as it is stored in fikes on node). On the other hand, using the push model and sending logs in batches almost guarantees losing a portion of logs during service crashes.

Therefore, it would be great to have separate configurations for OTLP logging and tracing. This would allow to configure tracing to send data to the collector via the push model, while retaining the previous behavior of outputting logs in TSKV format to STD-OUT or STD-ERR.

TertiumOrganum1 commented 2 weeks ago

We suggest (as a temporary solution) to add a config setting to optionally enable original logging while using OTLP logger. This way we could tweak logging config:

            loggers:
                default:
                    file_path: $log-location 
                    level: info
                    overflow_behavior: discard  
                opentracing:
                    file_path: /dev/null                  
                    overflow_behavior: discard   

and ignore logs in OTLP collector config.

So far, if we enable both loggers we get:

Cannot start component otlp-logger: You have registered both 'otlp-logger' component and 'default' logger in 'logging' component. Either disable default logger or otlp logger.
TertiumOrganum1 commented 2 weeks ago

Added PR as proposal for solution: https://github.com/userver-framework/userver/pull/679