open-telemetry / opentelemetry-cpp

The OpenTelemetry C++ Client
https://opentelemetry.io/
Apache License 2.0
817 stars 392 forks source link

Disable internal otel logging #2611

Closed Falmarri closed 2 months ago

Falmarri commented 3 months ago

Is it possible to disable the otel error logs? Specifically, the log messages such as

[Error] File: /opentelemetry-cpp/exporters/otlp/src/otlp_http_exporter.cc:106 [OTLP HTTP Client] ERROR: Export 357 trace span(s) error: 1

I can't seem to find any settings to set the log level of these or turn them off

lalitb commented 3 months ago

You can add a custom log-handler, with custom processing that could be to ignore the error logs - https://github.com/open-telemetry/opentelemetry-cpp/blob/611caa786ef2401bbf79d37e73cc9144993f9876/sdk/include/opentelemetry/sdk/common/global_log_handler.h#L107 Though recommendation would be to fix the error :)

Falmarri commented 3 months ago

The error happens when our collector is overloaded. Not much I can do about that, and it's not something clients can do anything about or should even know.

And just dumping data to stdout from a library seems very problematic. A tracing library should NEVER impact functionality of an application no matter what, even if it's not able to export spans. Dumping random stuff to stdout is very likely to cause actual problems. At the very least it should go to stderr by default, but even then, having a library write logs without explicit configuration seems very questionable to me.

marcalff commented 3 months ago

For an application using opentelemetry-cpp, we (speaking for the app, not as a maintainer) decided to implement a custom handler, and implement an extra log level setting : SILENT, by printing nothing in the custom log handler.

Having a SILENT log level supported in opentelemetry-cpp natively would help and make things simpler.

The motivation for silencing errors entirely is in particular during tests, because errors printed from opentelemetry-cpp in stdout affect the test output, causing functional tests of the app to fail even when the app is working normally.

Whether using a SILENT log level in production is desirable or not is up to the application, but at least I think (now speaking as a maintainer) that opentelemetry-cpp should support log level SILENT.

For robustness reasons, assuming one day we discover a crashing bug in an error code path (say, crashing while printing more data in the error log), having the possibility to disable this crashing code path would also allow the application to survive in a degraded mode, at least as a work around: defense in depth.

Falmarri commented 3 months ago

Can I suggest making silent the default? Like you said, content on stdout is bad for tests, but it's even worse for actual production code. Not everything using tracing is a daemon that just logs to stdout. My app has a small portion that can be used as part of a unix pipeline doing ETL work and having random stuff on stdout is not something I can even remotely deal with.

marcalff commented 3 months ago

The spec does not even enumerate possible values for OTEL_LOG_LEVEL, and implementation varies per language:

I think we can have a LogLevel::None, at least it will be consistent with .NET.

Note that opentelemetry-cpp does not support OTEL_LOG_LEVEL either, this probably should be fixed.

marcalff commented 3 months ago

At the very least it should go to stderr by default.

Indeed, having the default log handler print to std::cout is a poor choice.

lalitb commented 3 months ago

Indeed, having the default log handler print to std::cout is a poor choice.

Yes, it was me, probably writing this while feeling 😪 . The options as of now are:

marcalff commented 3 months ago

I think it is safe to:

@lalitb If this sounds good, I can make a PR for it.

lalitb commented 3 months ago

Yes, I think we should fix the code. Don't think it is feasible to do it withot breaking SDK ABI. Thanks :)