-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
[processor/filter] Add functionality to allow filtering on SeverityNumber #13002
[processor/filter] Add functionality to allow filtering on SeverityNumber #13002
Conversation
3628f2a
to
caae310
Compare
processor/filterprocessor/README.md
Outdated
e.g. if this is "WARN", all log records with "WARN" severity and above (WARN[2-4], ERROR[2-4], FATAL[2-4]) are matched. | ||
The list of valid severities that may be used for this option can be found [here](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/data-model.md#displaying-severity) | ||
By default, logs with undefined severity are not matched. | ||
- `match_undefined_severity`: MatchUndefinedSeverity defines whether to match logs with undefined severity or not when using the `min_severity` matching option. If `min_severity` is not specified, this option does nothing. If `match_undefined_severity` is set to true, log records with no severity will be matched. If set to false, log records with no severity will not be matched. |
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'm not sure we need this option. The data model has an opinion on where "undefined" severity ranks vs other severities levels, so I think we should just support it according to that ordering.
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 was really on the fence about this one, but IMO it's really useful from a user perspective, because "undefined" severity can come up in exceptional circumstances, e.g. I set up a filelog recevier, with a severity parsing operator. But maybe I mess up and typo one of the severities or something.
Now I'm dropping logs, because they are now undefined severity and I'm filtering everything below "info" level. But maybe those logs are actually really important, just the severity can't be parsed.
I think it's really nice to have this option when you need to be more fault-tolerant.
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 can appreciate the use case, but I don't think we should have a separate setting. What do you think about reworking the config a bit?
log_severity:
min: INFO
match_undefined: true
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.
Reworked the config as suggested; I used severity_number
instead of log_severity
to avoid stuttering and potential confusion with severity_texts.
e6a3169
to
71b787f
Compare
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
@boostchicken, do you want to take a look as code owner? |
71b787f
to
fd5a185
Compare
fd5a185
to
c402a2c
Compare
Description:
min_severity
is defined.Link to tracking Issue: #12959
Testing:
Documentation:
min_severity
andmatch_undefined_severity
fields