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

Does Log API and SDK need a way to enable log level and query for it #3185

Open
tigrannajaryan opened this issue Feb 7, 2023 · 7 comments
Open
Assignees
Labels
spec:logs Related to the specification/logs directory

Comments

@tigrannajaryan
Copy link
Member

This was raised by @reyang here and Otel C++ has a need for this capability. @open-telemetry/cpp-maintainers can you add more details on what exactly you need?

A couple references from other logging APIs that can be useful:

  • New Go slog package provides Enabled(level) func to check if the level is enabled with the current Logger (the frontend). The Logger checks with the Handler (the backend) whether the level is enabled before the log record is emitted.
  • In log4j2 AFAIK you can define filters by levels.
  • Python logging does the opposite, it allows to set the level of the Handler instead of asking if the particular level is to be produced.

The important question we need to answer now is if this needs to be part of our low-level Log API/SDK, since we are planning to stabilize it.

@tigrannajaryan tigrannajaryan added the spec:logs Related to the specification/logs directory label Feb 7, 2023
@tigrannajaryan
Copy link
Member Author

@open-telemetry/specs-logs-approvers please comment.

@jack-berg
Copy link
Member

Right now there is no concept of log level in the log SDK. Its not something you set on the provider, or configure at the logger level, and there is no mechanism that filters log records based on level.

This is really a question of whether log level should have some function in the SDK other than just being a field like any other.

If the API is to be used for purposes outside of writing appenders, then checking the level and having the SDK filter on level is an important capability. However, we've affirmed (multiple times) that we're not building a user facing logging API, so we'd need to reverse that decision.

Also, it seems reasonable to add these log level capabilities in the API / SDK in a backwards compatible way. I don't think this issue blocks stabilization.

@lalitb
Copy link
Member

lalitb commented Feb 8, 2023

Should have replied this comment here, pasting it now:

From performance prospective in C++, it is important to check the log-level at the Logging API level before we prepare the log-data, and invoke the actual logging. The application owner should be able to configure the log-level as the SDK configuration, which could be validated in the library using (say)Logger.IsEnabled(loglevel) before invoking logging. The configured log-level may also be used in LogProcessor/Exporter for another layer of filtering. Even with a logging framework, it is important to have this validation supported at logging backend API.

@reyang
Copy link
Member

reyang commented Feb 8, 2023

Also, it seems reasonable to add these log level capabilities in the API / SDK in a backwards compatible way. I don't think this issue blocks stabilization.

+1 on this can be an additive change

@jkwatson
Copy link
Contributor

jkwatson commented Feb 9, 2023

I suggest that a logging API for C++ should not be a part of otel proper. This should be a separate effort, divorced from the standard otel log appender API.

@jack-berg
Copy link
Member

Even with a logging framework, it is important to have this validation supported at logging backend API.

@lalitb can you elaborate on why this is required if we do in fact only deliver an "appender" or "logging backend" API? In the logging frameworks I've worked with, logs with a disabled log level never make it to the "appender" stage - they're filtered out upstream in the log framework. This makes it unnecessary to check if the particular log level is enabled.

@lalitb
Copy link
Member

lalitb commented Feb 10, 2023

@lalitb can you elaborate on why this is required if we do in fact only deliver an "appender" or "logging backend" API? In the logging frameworks I've worked with, logs with a disabled log level never make it to the "appender" stage - they're filtered out upstream in the log framework. This makes it unnecessary to check if the particular log level is enabled.

Yes agree it doesn't make sense to have it on "logging backend" API, as the logs would be already filtered at logging framework. Just want to emphasize on enabling setting logging-level in the SDK which can be used in (future) Logging API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec:logs Related to the specification/logs directory
Projects
None yet
Development

No branches or pull requests

6 participants