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.

Sign up for GitHub

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

feat(processors.enum): Add fields and tags config options #13824

Closed
wants to merge 2 commits into from
Closed

feat(processors.enum): Add fields and tags config options #13824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e197512
feat(processors.enum): Add fields and tags config options
powersj Aug 23, 2023
a9721ce
make docs
powersj Aug 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions plugins/processors/enum/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,17 @@ See the [CONFIGURATION.md][CONFIGURATION.md] for more details.
## Name of the field to map. Globs accepted.
field = "status"

## List of fields to appply mapping to. Globs accepted. Use either field or
## fields, but not both.
# fields = []

## Name of the tag to map. Globs accepted.
# tag = "status"

## List of tags to appply mapping to. Globs accepted. Use either tag or
## tags, but not both.
# tags = []

## Destination tag or field to be used for the mapped value. By default the
## source tag or field is used, overwriting the original value.
dest = "status_code"
Expand Down
31 changes: 27 additions & 4 deletions plugins/processors/enum/enum.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ type EnumMapper struct {

type Mapping struct {
Tag string
Tags []string
Field string
Fields []string
Comment on lines 25 to +28
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we please add add TOML annotations here and deprecate the singular options in favor of the plural ones?

Dest string
Default interface{}
ValueMappings map[string]interface{}
Expand All @@ -37,17 +39,38 @@ func (mapper *EnumMapper) Init() error {
mapper.FieldFilters = make(map[string]filter.Filter)
mapper.TagFilters = make(map[string]filter.Filter)
for _, mapping := range mapper.Mappings {
if mapping.Field != "" && len(mapping.Fields) != 0 {
return fmt.Errorf("use field or fields, but not both")
}
if mapping.Field != "" {
fieldFilter, err := filter.NewIncludeExcludeFilter([]string{mapping.Field}, nil)
if err != nil {
return fmt.Errorf("failed to create new field filter: %w", err)
return fmt.Errorf("failed to create new field filter from field: %w", err)
}
mapper.FieldFilters[mapping.Field] = fieldFilter
}
Comment on lines 45 to 51
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about just adding the single Field to the Fields array?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do agree with @Hipska. With the deprecation this would be the best way...

if len(mapping.Fields) != 0 {
fieldFilter, err := filter.NewIncludeExcludeFilter(mapping.Fields, nil)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use a simple filter instead of an IncludeExcludeFilter here.

if err != nil {
return fmt.Errorf("failed to create new field filter from fields: %w", err)
}
mapper.FieldFilters[mapping.Field] = fieldFilter
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why you use a map for the filter... The key for the map is either empty (if no field is defined) or has the field name, i.e. the map will only contain a single entry. I think you should store the filter directly instead of using a map.

}

if mapping.Tag != "" && len(mapping.Tags) != 0 {
return fmt.Errorf("use tag or tags, but not both")
}
if mapping.Tag != "" {
tagFilter, err := filter.NewIncludeExcludeFilter([]string{mapping.Tag}, nil)
if err != nil {
return fmt.Errorf("failed to create new tag filter: %w", err)
return fmt.Errorf("failed to create new tag filter from tag: %w", err)
}
mapper.TagFilters[mapping.Tag] = tagFilter
}
if len(mapping.Tags) != 0 {
tagFilter, err := filter.NewIncludeExcludeFilter(mapping.Tags, nil)
if err != nil {
return fmt.Errorf("failed to create new tag filter from tags: %w", err)
}
mapper.TagFilters[mapping.Tag] = tagFilter
}
Expand All @@ -68,10 +91,10 @@ func (mapper *EnumMapper) applyMappings(metric telegraf.Metric) telegraf.Metric
newTags := make(map[string]string)

for _, mapping := range mapper.Mappings {
if mapping.Field != "" {
if mapping.Field != "" || len(mapping.Fields) != 0 {
mapper.fieldMapping(metric, mapping, newFields)
}
if mapping.Tag != "" {
if mapping.Tag != "" || len(mapping.Tags) != 0 {
mapper.tagMapping(metric, mapping, newTags)
}
Comment on lines -71 to 99
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As said before, I would prefer to create a filter for tags and fields and then simply check for the filter being not-nil here. I think this will simplify the code...

}
Expand Down
32 changes: 32 additions & 0 deletions plugins/processors/enum/enum_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,3 +196,35 @@ func TestTagGlobMatching(t *testing.T) {

assertTagValue(t, "glob", "tag", tags)
}

func TestFieldsAndTags(t *testing.T) {
mapper := EnumMapper{Mappings: []Mapping{
{
Tags: []string{"tag", "duplicate_tag"},
Fields: []string{"string_value", "duplicate_string_value"},
ValueMappings: map[string]interface{}{
"test": "0",
"tag_value": "1",
},
},
}}
require.NoError(t, mapper.Init())

tags := calculateProcessedTags(mapper, createTestMetric())
assertTagValue(t, "1", "tag", tags)
assertTagValue(t, "1", "duplicate_tag", tags)

fields := calculateProcessedValues(mapper, createTestMetric())
assertFieldValue(t, "0", "string_value", fields)
assertFieldValue(t, "0", "duplicate_string_value", fields)
}

func TestTagAndTagsError(t *testing.T) {
mapper := EnumMapper{Mappings: []Mapping{{Tag: "*", Tags: []string{"Foo"}, ValueMappings: map[string]interface{}{"tag_value": "glob"}}}}
require.Error(t, mapper.Init())
}

func TestFieldAndFieldsError(t *testing.T) {
mapper := EnumMapper{Mappings: []Mapping{{Field: "*", Fields: []string{"Bar"}, ValueMappings: map[string]interface{}{"tag_value": "glob"}}}}
require.Error(t, mapper.Init())
}
8 changes: 8 additions & 0 deletions plugins/processors/enum/sample.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,17 @@
## Name of the field to map. Globs accepted.
field = "status"

## List of fields to appply mapping to. Globs accepted. Use either field or
## fields, but not both.
# fields = []

## Name of the tag to map. Globs accepted.
# tag = "status"

## List of tags to appply mapping to. Globs accepted. Use either tag or
## tags, but not both.
# tags = []

## Destination tag or field to be used for the mapped value. By default the
## source tag or field is used, overwriting the original value.
dest = "status_code"
Expand Down