-
Notifications
You must be signed in to change notification settings - Fork 17
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
add log_level configuration #121
base: main
Are you sure you want to change the base?
Conversation
This is to support the env variable OTEL_LOG_LEVEL. Fixes open-telemetry#120 Signed-off-by: Alex Boten <[email protected]>
Related: Until the spec defines an enum for log levels, opentelemetry-configuration can only represent a log level as a string, to be interpreted (differently) by each SDK implementation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
This is the best we can do now, because the spec does not define an enum for log levels.
examples/kitchen-sink.yaml
Outdated
@@ -15,6 +15,11 @@ file_format: "0.1" | |||
# Environment variable: OTEL_SDK_DISABLED | |||
disabled: false | |||
|
|||
# Configure the log level used by the SDK logger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the SDK logger? I think you mean to say the internal logger used internally by the SDK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know but I think that definition is lame. Can / should we do better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated, PTAL
@@ -11,6 +11,9 @@ | |||
"disabled": { | |||
"type": ["boolean", "null"] | |||
}, | |||
"log_level": { | |||
"type": ["string", "null"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have an enum based on a subset of the SeverityNumber enum defined in the proto repo? https://github.com/open-telemetry/opentelemetry-proto/blob/4f69356d853029975649c3f38b06fc77d77975fc/opentelemetry/proto/logs/v1/logs.proto#L86-L110
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could, though without a resolution on open-telemetry/opentelemetry-specification#2039, we may be jumping ahead of the spec
This is to support the env variable OTEL_LOG_LEVEL.
Fixes #120