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

Could internal log level be set by runtime? #1138

Closed
owent opened this issue Dec 14, 2021 · 6 comments · Fixed by #1147
Closed

Could internal log level be set by runtime? #1138

owent opened this issue Dec 14, 2021 · 6 comments · Fixed by #1147
Assignees

Comments

@owent
Copy link
Member

owent commented Dec 14, 2021

Is your feature request related to a problem?

We usually need debug log when testing and integrating opentelemetry-cpp, and just need error log after that.But now the internal log level can only be set when compiling, which is usual WARNING.

Describe the solution you'd like

Maybe add GlobalLogHandler::SetLogLevel(...) or add log_level to GlobalLogHandler::SetLogHandler(...) ?

@lalitb
Copy link
Member

lalitb commented Dec 14, 2021

The original intention of the internal logger was to configure the custom error handler as specified in specs ( https://github.com/open-telemetry/opentelemetry-specification/blob/v1.6.x/specification/error-handling.md#configuring-error-handlers ) and not to use it for debugging/testing. So, the levels were kept compile-only, to ensure we have header-only logging.
Said that I don't see any issue extending it further to have the levels configurable at runtime if it helps to debug. Would just be cautious not to make it as another full-fledged logger implementation ( along with OTel logger API ).

@ThomsonTan
Copy link
Contributor

If the SDK allows user to register customized error handlers, then they can do any whatever filtering there like the dynamic log level, is this correct?

@lalitb
Copy link
Member

lalitb commented Dec 14, 2021

If the SDK allows user to register customized error handlers, then they can do any whatever filtering there like the dynamic log level, is this correct?

Yes, but the custom handler will only receive the logs at the level configured during compile time.

@owent
Copy link
Member Author

owent commented Dec 15, 2021

The original intention of the internal logger was to configure the custom error handler as specified in specs ( https://github.com/open-telemetry/opentelemetry-specification/blob/v1.6.x/specification/error-handling.md#configuring-error-handlers ) and not to use it for debugging/testing. So, the levels were kept compile-only, to ensure we have header-only logging. Said that I don't see any issue extending it further to have the levels configurable at runtime if it helps to debug. Would just be cautious not to make it as another full-fledged logger implementation ( along with OTel logger API ).

The java example in https://github.com/open-telemetry/opentelemetry-specification/blob/v1.6.x/specification/error-handling.md#configuring-error-handlers seems to use a global variable io.opentelemetry.level to control the log level. Could we use the same way or use another static variable to set log level here(Just like the global log handle)? I think we can still keep header-only logging by this way.

@lalitb
Copy link
Member

lalitb commented Dec 15, 2021

Could we use the same way or use another static variable to set log level here(Just like the global log handle)? I think we can still keep header-only logging by this way.

Yes, that would be good. Thanks.

@owent
Copy link
Member Author

owent commented Dec 15, 2021

Could we use the same way or use another static variable to set log level here(Just like the global log handle)? I think we can still keep header-only logging by this way.

Yes, that would be good. Thanks.

Could you please assign this to me? Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants