Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable internal otel logging #2611

Closed
Falmarri opened this issue Mar 27, 2024 · 9 comments · Fixed by #2622
Closed

Disable internal otel logging #2611

Falmarri opened this issue Mar 27, 2024 · 9 comments · Fixed by #2622
Assignees
Labels
bug Something isn't working triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@Falmarri
Copy link

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

@Falmarri Falmarri added the bug Something isn't working label Mar 27, 2024
@github-actions github-actions bot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Mar 27, 2024
@lalitb
Copy link
Member

lalitb commented Mar 27, 2024

You can add a custom log-handler, with custom processing that could be to ignore the error logs -

static inline void SetLogHandler(nostd::shared_ptr<LogHandler> eh) noexcept

Though recommendation would be to fix the error :)

@Falmarri
Copy link
Author

Falmarri commented Mar 28, 2024

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
Copy link
Member

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.

@marcalff marcalff self-assigned this Mar 28, 2024
@Falmarri
Copy link
Author

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

lalitb commented Mar 28, 2024

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:

  • use custom log handler. And don't rely on DefaultLogHandler for production usage.
  • Rebuild otel-cpp sdk with macro OTEL_INTERNAL_LOG_LEVEL as -1 or anything less than 0.
  • redirect application stdout to /dev/null.

@marcalff
Copy link
Member

I think it is safe to:

  • Change the DefaultLogHandler to use stderr instead, and document this as a SDK important change. I would be surprised if anyone considers this breaking, real applications most likely use their own handler anyway.
  • Re number log levels starting with None=0, Error=1, etc, and also document this as an important change. This changes the SDK ABI in case someone links dynamically with the SDK, which we do not support anyway.

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

@lalitb
Copy link
Member

lalitb commented Mar 28, 2024

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

marcalff added a commit to marcalff/opentelemetry-cpp that referenced this issue Mar 28, 2024
@marcalff marcalff added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants