[processor/sumologicschema]: allow aggregating attributes with given name patterns #871
Conversation
aboguszewski-sumo
force-pushed
the
sumoschema-aggregate-patterns
branch
2 times, most recently
from
e3f56a2 to
0a9adc6
Compare
github-actions
bot
added
the
documentation
aboguszewski-sumo
force-pushed
the
sumoschema-aggregate-patterns
branch
from
0a9adc6 to
d3edaff
Compare
|
|
||
| for j := 0; j < resourceLogs.ScopeLogs().Len(); j++ { | ||
| scopeLogs := resourceLogs.ScopeLogs().At(j) | ||
| for k := 0; k < scopeLogs.LogRecords().Len(); k++ { | ||
| err := proc.aggregateAttributes(scopeLogs.LogRecords().At(k).Attributes()) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
idk what is the exact purpose for this feature, but maybe by default we can only modify resource attributes and log/metric/span-level attributes should be configurable.
| func pairToAggregation(pair *aggregationPair) (*aggregation, error) { | ||
| regexes := []*regexp.Regexp{} | ||
|
|
||
| for i := 0; i < len(pair.Patterns); i++ { | ||
| // We do not support regexes - only wildcards (*). Escape all regex special characters. | ||
| regexStr := regexp.QuoteMeta(pair.Patterns[i]) | ||
|
|
||
| // Replace all wildcards (after escaping they are "\*") with grouped regex wildcard ("(.*)") | ||
| regexStrWithWildcard := strings.Replace(regexStr, "\\*", "(.*)", -1) | ||
|
|
||
| regex, err := regexp.Compile(regexStrWithWildcard) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| regexes = append(regexes, regex) | ||
| } | ||
|
|
||
| return &aggregation{attribute: pair.Attribute, patternRegexes: regexes}, nil | ||
| } |
There was a problem hiding this comment.
This does not check for exact matches, for example if user specifies pattern pod_*, we will match strings like opod_123. We should probably discard that, shouldn't we?
There was a problem hiding this comment.
Yes, we should check for exact matches. Also see my comment here: #866 (comment).
| // Join all substrings caught by wildcards into one string, | ||
| // this string will be the name of this key in the new map. | ||
| // TODO: Potential name conflict to resolve, eg.: | ||
| // pod_*_bar_* matches pod_foo_bar_baz | ||
| // pod2_*_bar_* matches pod2_foo_bar_baz | ||
| // both will be renamed to foo_baz | ||
| name := strings.Join(match[1:], "_") | ||
| names = append(names, name) | ||
| val := pcommon.NewValueEmpty() | ||
| value.CopyTo(val) | ||
| attrs = append(attrs, val) |
There was a problem hiding this comment.
As mentioned in the comment. This seems to be a general problem with naming, which I'm not sure how to solve.
aboguszewski-sumo
force-pushed
the
sumoschema-aggregate-patterns
branch
from
d3edaff to
a79284d
Compare
| regexes := []*regexp.Regexp{} | ||
|
|
||
| for i := 0; i < len(pair.Patterns); i++ { | ||
| // We do not support regexes - only wildcards (*). Escape all regex special characters. |
There was a problem hiding this comment.
We should either support regexes and require a named capture group, or only support prefixes. See: #866 (comment)
There was a problem hiding this comment.
There's a problem with naming in that case. But yes, now that I think about it, we can just force the user to specify at least one capture group
aboguszewski-sumo
force-pushed
the
sumoschema-aggregate-patterns
branch
3 times, most recently
from
419bfd7 to
95e3f1c
Compare
|
PR reworked. |
aboguszewski-sumo
force-pushed
the
sumoschema-aggregate-patterns
branch
from
95e3f1c to
51a384d
Compare
aboguszewski-sumo
force-pushed
the
sumoschema-aggregate-patterns
branch
from
51a384d to
c9cfe52
Compare
Updates #866
Done. This is a basic version, which supports only prefixes.