Skip to content

Commit

Permalink
[pkg/stanza] Fix serverity range unmarshaling (#16339)
Browse files Browse the repository at this point in the history 
This functionality was broken during refactoring of the unmarshaling logic.

Previously, various yaml types could be used to represent a severity level.
This was designed to be as open as possible to future implementations. However,
it is now clear that only some keys are valid anyways. Namely, string and ints
that represent OTel's native severity levels. Both strings and ints are now treated
as strings when unmarshaling, which is fully compatible with the sev map lookup code.
  • Loading branch information
@djaglowski
djaglowski authored Nov 17, 2022
1 parent 47afa8a commit cd1fc02
Show file tree
Hide file tree
Showing 11 changed files with 131 additions and 109 deletions.
16 changes: 16 additions & 0 deletions .chloggen/pkg-stanza-sev-range-fix.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: pkg/stanza

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Fix severity range unmarshaling

# One or more tracking issues related to the change
issues: [16339]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:
2 changes: 1 addition & 1 deletion pkg/stanza/operator/helper/parser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ func NewTestParserConfig() ParserConfig {
expect.TimeParser = &tp

sp := NewSeverityConfig()
sp.Mapping = map[interface{}]interface{}{
sp.Mapping = map[string]interface{}{
"info": "3xx",
"warn": "4xx",
}
Expand Down
37 changes: 5 additions & 32 deletions pkg/stanza/operator/helper/severity_builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,34 +29,7 @@ func getBuiltinMapping(name string) severityMap {
switch name {
case "none":
return map[string]entry.Severity{}
case "aliases":
return map[string]entry.Severity{
"trace": entry.Trace,
"trace2": entry.Trace2,
"trace3": entry.Trace3,
"trace4": entry.Trace4,
"debug": entry.Debug,
"debug2": entry.Debug2,
"debug3": entry.Debug3,
"debug4": entry.Debug4,
"info": entry.Info,
"info2": entry.Info2,
"info3": entry.Info3,
"info4": entry.Info4,
"warn": entry.Warn,
"warn2": entry.Warn2,
"warn3": entry.Warn3,
"warn4": entry.Warn4,
"error": entry.Error,
"error2": entry.Error2,
"error3": entry.Error3,
"error4": entry.Error4,
"fatal": entry.Fatal,
"fatal2": entry.Fatal2,
"fatal3": entry.Fatal3,
"fatal4": entry.Fatal4,
}
case "otel":
case "otel", "aliases":
return map[string]entry.Severity{
"trace": entry.Trace,
"1": entry.Trace,
Expand Down Expand Up @@ -149,9 +122,9 @@ func NewSeverityConfig() SeverityConfig {

// SeverityConfig allows users to specify how to parse a severity from a field.
type SeverityConfig struct {
ParseFrom *entry.Field `mapstructure:"parse_from,omitempty"`
Preset string `mapstructure:"preset,omitempty"`
Mapping map[interface{}]interface{} `mapstructure:"mapping,omitempty"`
ParseFrom *entry.Field `mapstructure:"parse_from,omitempty"`
Preset string `mapstructure:"preset,omitempty"`
Mapping map[string]interface{} `mapstructure:"mapping,omitempty"`
}

// Build builds a SeverityParser from a SeverityConfig
Expand Down Expand Up @@ -200,7 +173,7 @@ func validateSeverity(severity interface{}) (entry.Severity, error) {
}

func isRange(value interface{}) (int, int, bool) {
rawMap, ok := value.(map[interface{}]interface{})
rawMap, ok := value.(map[string]interface{})
if !ok {
return 0, 0, false
}
Expand Down
122 changes: 75 additions & 47 deletions pkg/stanza/operator/helper/severity_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ type severityTestCase struct {
name string
sample interface{}
mappingSet string
mapping map[interface{}]interface{}
mapping map[string]interface{}
buildErr bool
parseErr bool
expected entry.Severity
Expand Down Expand Up @@ -77,7 +77,7 @@ func validMappingKeyCases() []severityTestCase {
severityTestCase{
name: k,
sample: "my_custom_value",
mapping: map[interface{}]interface{}{k: "my_custom_value"},
mapping: map[string]interface{}{k: "my_custom_value"},
expected: v,
})
}
Expand Down Expand Up @@ -142,19 +142,19 @@ func otlpSevCases() []severityTestCase {
return cases
}

func TestSeverityParser(t *testing.T) {
allTheThingsMap := map[interface{}]interface{}{
"info": "3xx",
"error3": "4xx",
"debug4": "5xx",
"trace2": []interface{}{
"ttttttracer",
[]byte{100, 100, 100},
map[interface{}]interface{}{"min": 1111, "max": 1234},
},
"fatal2": "",
}
var allTheThingsMap = map[string]interface{}{
"info": "3xx",
"error3": "4xx",
"debug4": "5xx",
"trace2": []interface{}{
"ttttttracer",
map[string]interface{}{"min": 1111, "max": 1234},
},
"12": "infooo",
"fatal2": "",
}

func TestSeverityParser(t *testing.T) {
testCases := []severityTestCase{
{
name: "unknown",
Expand Down Expand Up @@ -201,127 +201,127 @@ func TestSeverityParser(t *testing.T) {
{
name: "custom-string",
sample: "NOOOOOOO",
mapping: map[interface{}]interface{}{"error": "NOOOOOOO"},
mapping: map[string]interface{}{"error": "NOOOOOOO"},
expected: entry.Error,
},
{
name: "custom-string-caps-key",
sample: "NOOOOOOO",
mapping: map[interface{}]interface{}{"ErRoR": "NOOOOOOO"},
mapping: map[string]interface{}{"ErRoR": "NOOOOOOO"},
expected: entry.Error,
},
{
name: "custom-int",
sample: 1234,
mapping: map[interface{}]interface{}{"error": 1234},
mapping: map[string]interface{}{"error": 1234},
expected: entry.Error,
},
{
name: "mixed-list-string",
sample: "ThiS Is BaD",
mapping: map[interface{}]interface{}{"error": []interface{}{"NOOOOOOO", "this is bad", 1234}},
mapping: map[string]interface{}{"error": []interface{}{"NOOOOOOO", "this is bad", 1234}},
expected: entry.Error,
},
{
name: "custom-float64",
sample: float64(6),
mapping: map[interface{}]interface{}{"error": 6},
mapping: map[string]interface{}{"error": 6},
expected: entry.Error,
},
{
name: "mixed-list-int",
sample: 1234,
mapping: map[interface{}]interface{}{"error": []interface{}{"NOOOOOOO", "this is bad", 1234}},
mapping: map[string]interface{}{"error": []interface{}{"NOOOOOOO", "this is bad", 1234}},
expected: entry.Error,
},
{
name: "numbered-level",
sample: "critical",
mapping: map[interface{}]interface{}{"error2": "critical"},
mapping: map[string]interface{}{"error2": "critical"},
expected: entry.Error2,
},
{
name: "override-standard",
sample: "error",
mapping: map[interface{}]interface{}{"error3": []interface{}{"error"}},
mapping: map[string]interface{}{"error3": []interface{}{"error"}},
expected: entry.Error3,
},
{
name: "level-unfound",
sample: "not-in-the-list-but-thats-ok",
mapping: map[interface{}]interface{}{"error4": []interface{}{"hey!", 1234}},
mapping: map[string]interface{}{"error4": []interface{}{"hey!", 1234}},
expected: entry.Default,
},
{
name: "in-range",
sample: 123,
mapping: map[interface{}]interface{}{"error": map[interface{}]interface{}{"min": 120, "max": 125}},
mapping: map[string]interface{}{"error": map[string]interface{}{"min": 120, "max": 125}},
expected: entry.Error,
},
{
name: "in-range-min",
sample: 120,
mapping: map[interface{}]interface{}{"error": map[interface{}]interface{}{"min": 120, "max": 125}},
mapping: map[string]interface{}{"error": map[string]interface{}{"min": 120, "max": 125}},
expected: entry.Error,
},
{
name: "in-range-max",
sample: 125,
mapping: map[interface{}]interface{}{"error": map[interface{}]interface{}{"min": 120, "max": 125}},
mapping: map[string]interface{}{"error": map[string]interface{}{"min": 120, "max": 125}},
expected: entry.Error,
},
{
name: "out-of-range-min-minus",
sample: 119,
mapping: map[interface{}]interface{}{"error": map[interface{}]interface{}{"min": 120, "max": 125}},
mapping: map[string]interface{}{"error": map[string]interface{}{"min": 120, "max": 125}},
expected: entry.Default,
},
{
name: "out-of-range-max-plus",
sample: 126,
mapping: map[interface{}]interface{}{"error": map[interface{}]interface{}{"min": 120, "max": 125}},
mapping: map[string]interface{}{"error": map[string]interface{}{"min": 120, "max": 125}},
expected: entry.Default,
},
{
name: "range-out-of-order",
sample: 123,
mapping: map[interface{}]interface{}{"error": map[interface{}]interface{}{"min": 125, "max": 120}},
mapping: map[string]interface{}{"error": map[string]interface{}{"min": 125, "max": 120}},
expected: entry.Error,
},
{
name: "Http2xx-hit",
sample: 201,
mapping: map[interface{}]interface{}{"error": "2xx"},
mapping: map[string]interface{}{"error": "2xx"},
expected: entry.Error,
},
{
name: "Http2xx-miss",
sample: 301,
mapping: map[interface{}]interface{}{"error": "2xx"},
mapping: map[string]interface{}{"error": "2xx"},
expected: entry.Default,
},
{
name: "Http3xx-hit",
sample: 301,
mapping: map[interface{}]interface{}{"error": "3xx"},
mapping: map[string]interface{}{"error": "3xx"},
expected: entry.Error,
},
{
name: "Http4xx-hit",
sample: "404",
mapping: map[interface{}]interface{}{"error": "4xx"},
mapping: map[string]interface{}{"error": "4xx"},
expected: entry.Error,
},
{
name: "Http5xx-hit",
sample: 555,
mapping: map[interface{}]interface{}{"error": "5xx"},
mapping: map[string]interface{}{"error": "5xx"},
expected: entry.Error,
},
{
name: "Http-All",
sample: "301",
mapping: map[interface{}]interface{}{"debug": "2xx", "info": "3xx", "error": "4xx", "warn": "5xx"},
mapping: map[string]interface{}{"debug": "2xx", "info": "3xx", "error": "4xx", "warn": "5xx"},
expected: entry.Info,
},
{
Expand All @@ -330,12 +330,6 @@ func TestSeverityParser(t *testing.T) {
mapping: allTheThingsMap,
expected: entry.Trace2,
},
{
name: "all-the-things-bytes",
sample: []byte{100, 100, 100},
mapping: allTheThingsMap,
expected: entry.Trace2,
},
{
name: "all-the-things-empty",
sample: "",
Expand Down Expand Up @@ -480,6 +474,45 @@ func (tc severityTestCase) run(parseFrom entry.Field) func(*testing.T) {
}
}

func TestBuildCustomMapping(t *testing.T) {
t.Parallel()

expected := severityMap{
"300": entry.Info,
"301": entry.Info,
"399": entry.Info,
"400": entry.Error3,
"410": entry.Error3,
"499": entry.Error3,
"500": entry.Debug4,
"555": entry.Debug4,
"599": entry.Debug4,
"ttttttracer": entry.Trace2,
"1111": entry.Trace2,
"1200": entry.Trace2,
"1234": entry.Trace2,
"infooo": entry.Info4, // 12
"": entry.Fatal2,
}

parseFrom := entry.NewBodyField()
cfg := &SeverityConfig{
ParseFrom: &parseFrom,
Mapping: allTheThingsMap,
}

severityParser, err := cfg.Build(testutil.Logger(t))
require.NoError(t, err)

for k, v := range expected {
sev, _, err := severityParser.Mapping.find(k)
t.Run(k, func(t *testing.T) {
require.NoError(t, err)
require.Equal(t, v, sev)
})
}
}

func TestUnmarshalSeverityConfig(t *testing.T) {
operatortest.ConfigUnmarshalTests{
DefaultConfig: newHelpersConfig(),
Expand All @@ -490,12 +523,7 @@ func TestUnmarshalSeverityConfig(t *testing.T) {
Expect: func() *helpersConfig {
c := newHelpersConfig()
c.Severity = NewSeverityConfig()
c.Severity.Mapping = map[interface{}]interface{}{
"critical": "5xx",
"error": "4xx",
"info": "3xx",
"debug": "2xx",
}
c.Severity.Mapping = allTheThingsMap
return c
}(),
},
Expand Down
11 changes: 8 additions & 3 deletions pkg/stanza/operator/helper/testdata/severity.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,15 @@ mapping:
type: helpers_test
severity:
mapping:
critical: 5xx
error: 4xx
info: 3xx
debug: 2xx
error3: 4xx
debug4: 5xx
trace2:
- "ttttttracer"
- min: 1111
max: 1234
fatal2: ""
12: infooo
parse_from:
type: helpers_test
severity:
Expand Down
2 changes: 1 addition & 1 deletion pkg/stanza/operator/parser/json/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func TestConfig(t *testing.T) {
parseField := entry.NewBodyField("severity_field")
severityParser := helper.NewSeverityConfig()
severityParser.ParseFrom = &parseField
mapping := map[interface{}]interface{}{
mapping := map[string]interface{}{
"critical": "5xx",
"error": "4xx",
"info": "3xx",
Expand Down
2 changes: 1 addition & 1 deletion pkg/stanza/operator/parser/keyvalue/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func TestConfig(t *testing.T) {
parseField := entry.NewBodyField("severity_field")
severityField := helper.NewSeverityConfig()
severityField.ParseFrom = &parseField
mapping := map[interface{}]interface{}{
mapping := map[string]interface{}{
"critical": "5xx",
"error": "4xx",
"info": "3xx",
Expand Down
Loading

0 comments on commit cd1fc02

Please sign in to comment.