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

[processor/filter] Add functionality to allow filtering on SeverityNumber #13002

Merged
Prev Previous commit
Next Next commit
add ability to control matching undefined severity records
  • Loading branch information
BinaryFissionGames authored and djaglowski committed Aug 11, 2022
commit ef27a3d684d9c2794fbdc968d37e071c4dea14a7
6 changes: 6 additions & 0 deletions internal/coreinternal/processor/filterconfig/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,12 @@ type MatchProperties struct {
// e.g. if this is plog.SeverityNumberINFO, INFO, WARN, ERROR, and FATAL logs will match.
LogMinSeverity plog.SeverityNumber `mapstructure:"log_min_severity"`

// LogMatchUndefinedSeverity controls whether logs with "undefined" severity matches.
// If this is true, entries with undefined severity will match.
// This field is only applicable if LogMinSeverity is specified;
// If LogMinSeverity is not specified, this field does nothing.
LogMatchUndefinedSeverity bool `mapstructure:"log_match_undefined_severity"`

// MetricNames is a list of strings to match metric name against.
// A match occurs if metric name matches at least one item in the list.
// This field is optional.
Expand Down
2 changes: 1 addition & 1 deletion internal/coreinternal/processor/filterlog/filterlog.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func NewMatcher(mp *filterconfig.MatchProperties) (Matcher, error) {

var severityNumberMatcher Matcher
if mp.LogMinSeverity != plog.SeverityNumberUNDEFINED {
severityNumberMatcher = newSeverityNumberMatcher(mp.LogMinSeverity)
severityNumberMatcher = newSeverityNumberMatcher(mp.LogMinSeverity, mp.LogMatchUndefinedSeverity)
}

return &propertiesMatcher{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,21 @@ import (
// severtiyNumberMatcher is a Matcher that matches if the input log record has a severity higher than
// the minSeverityNumber.
type severityNumberMatcher struct {
matchUndefined bool
minSeverityNumber plog.SeverityNumber
}

func newSeverityNumberMatcher(sn plog.SeverityNumber) severityNumberMatcher {
func newSeverityNumberMatcher(minSeverity plog.SeverityNumber, matchUndefined bool) severityNumberMatcher {
return severityNumberMatcher{
minSeverityNumber: sn,
minSeverityNumber: minSeverity,
matchUndefined: matchUndefined,
}
}

func (snm severityNumberMatcher) MatchLogRecord(lr plog.LogRecord, _ pcommon.Resource, _ pcommon.InstrumentationScope) bool {
// We explicitly do not match UNDEFINED severity.
if lr.SeverityNumber() == plog.SeverityNumberUNDEFINED {
return false
return snm.matchUndefined
}

// If the log records severity is greater than or equal to the desired severity, it matches
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ import (

func TestSeverityMatcher_MatchLogRecord(t *testing.T) {
testCases := []struct {
name string
minSeverity plog.SeverityNumber
inputSeverity plog.SeverityNumber
matches bool
name string
minSeverity plog.SeverityNumber
matchUndefined bool
inputSeverity plog.SeverityNumber
matches bool
}{
{
name: "INFO matches if TRACE is min",
Expand Down Expand Up @@ -57,11 +58,18 @@ func TestSeverityMatcher_MatchLogRecord(t *testing.T) {
inputSeverity: plog.SeverityNumberUNDEFINED,
matches: false,
},
{
name: "UNDEFINED matches if matchUndefined is true",
minSeverity: plog.SeverityNumberUNDEFINED,
matchUndefined: true,
inputSeverity: plog.SeverityNumberUNDEFINED,
matches: false,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
matcher := newSeverityNumberMatcher(tc.minSeverity)
matcher := newSeverityNumberMatcher(tc.minSeverity, tc.matchUndefined)

r := pcommon.NewResource()
i := pcommon.NewInstrumentationScope()
Expand Down
3 changes: 3 additions & 0 deletions processor/filterprocessor/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ For logs:
e.g. if this is "INFO", all log records with "INFO" severity and above (WARN[2-4], ERROR[2-4], FATAL[2-4]) are matched.
If this option is specified, no logs with "DEFAULT" severity will be 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)
- `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.
Copy link
Member

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.

Copy link
Contributor Author

@BinaryFissionGames BinaryFissionGames Aug 8, 2022

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.

Copy link
Member

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

Copy link
Contributor Author

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.


For metrics:

Expand Down Expand Up @@ -104,9 +105,11 @@ processors:
- WARN[2-4]?
- ERROR[2-4]?
# Filter out logs below INFO (no DEBUG or TRACE level logs)
# log records
logs/severity_number:
include:
min_severity: "INFO"
match_undefined_severity: true
logs/bodies:
include:
match_type: regexp
Expand Down
16 changes: 11 additions & 5 deletions processor/filterprocessor/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,11 @@ type LogMatchProperties struct {
// this field is case-insensitive ("INFO" == "info")
MinSeverity logSeverity `mapstructure:"min_severity"`

// MatchUndefinedSeverity lets logs records with "unknown" severity match.
// This is only applied if MinSeverity is set.
// If MinSeverity is not set, this field is ignored, as fields are not matched based on severity.
MatchUndefinedSeverity bool `mapstructure:"match_undefined_severity"`

// LogBodies is a list of strings that the LogRecord's body field must match
// against.
LogBodies []string `mapstructure:"bodies"`
Expand All @@ -190,11 +195,12 @@ func (lmp LogMatchProperties) matchProperties() *filterconfig.MatchProperties {
Config: filterset.Config{
MatchType: filterset.MatchType(lmp.LogMatchType),
},
Resources: lmp.ResourceAttributes,
Attributes: lmp.RecordAttributes,
LogSeverityTexts: lmp.SeverityTexts,
LogBodies: lmp.LogBodies,
LogMinSeverity: lmp.MinSeverity.severityNumber(),
Resources: lmp.ResourceAttributes,
Attributes: lmp.RecordAttributes,
LogSeverityTexts: lmp.SeverityTexts,
LogBodies: lmp.LogBodies,
LogMinSeverity: lmp.MinSeverity.severityNumber(),
LogMatchUndefinedSeverity: lmp.MatchUndefinedSeverity,
}
}

Expand Down
3 changes: 2 additions & 1 deletion processor/filterprocessor/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,8 @@ func TestLoadingConfigBodyLogsRegexp(t *testing.T) {
// TestLoadingConfigMinSeverityNumberLogs tests loading testdata/config_logs_min_severity.yaml
func TestLoadingConfigMinSeverityNumberLogs(t *testing.T) {
testDataLogPropertiesInclude := &LogMatchProperties{
MinSeverity: logSeverity("INFO"),
MinSeverity: logSeverity("INFO"),
MatchUndefinedSeverity: true,
}

testDataLogPropertiesExclude := &LogMatchProperties{
Expand Down
24 changes: 24 additions & 0 deletions processor/filterprocessor/filter_processor_logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,18 @@ var (
{"log3"},
},
},
{
name: "includeMinSeverityFATAL+undefined",
inc: &LogMatchProperties{
LogMatchType: Regexp,
MinSeverity: logSeverity("FATAL"),
MatchUndefinedSeverity: true,
},
inLogs: testResourceLogs(inLogForSeverityNumber),
outLN: [][]string{
{"log4"},
},
},
{
name: "excludeMinSeverityINFO",
exc: &LogMatchProperties{
Expand All @@ -536,6 +548,18 @@ var (
{"log4"},
},
},
{
name: "excludeMinSeverityINFO+undefined",
exc: &LogMatchProperties{
LogMatchType: Regexp,
MinSeverity: logSeverity("INFO"),
MatchUndefinedSeverity: true,
},
inLogs: testResourceLogs(inLogForSeverityNumber),
outLN: [][]string{
{"log1"},
},
},
}
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ processors:
logs:
# any logs NOT matching filters are excluded from remainder of pipeline
# This filters out all logs below "INFO" level (the whole DEBUG and TRACE ranges)
# Logs with no defined severity will also be matched.
include:
min_severity: "INFO"
match_undefined_severity: true
filter/exclude:
logs:
# any logs matching filters are excluded from remainder of pipeline
Expand All @@ -18,9 +20,11 @@ processors:
filter/includeexclude:
logs:
# if both include and exclude are specified, include filters are applied first
# the following will only allow records with severity in the "INFO" and "WARN" ranges to pass
# the following will only allow records with severity in the "INFO" and "WARN" ranges to pass,
# as well as logs with undefined severity
include:
min_severity: "INFO"
match_undefined_severity: true

exclude:
min_severity: "ERROR"
Expand Down