Skip to content

Commit 1ac21d6

Browse files
braydonkTylerHelmuth
authored andcommitted
aggregateutil: allow filtering against empty attribute set (open-telemetry#35006)
**Description:** <Describe what has changed.> It is a valid use case to aggregate against an empty label set, which will functionally clear all attributes. This behaviour was removed in open-telemetry#33655, which simplified the check to `len() == 0`, which covers the case of the label set being `nil` and having 0 elements as the same scenario. However, these are not the same scenario and have different meanings. This PR reintroduces the original behaviour, but in a more efficient way by recognizing a label set with 0 elements and clearing the attributes, which would be the logical conclusion after running the filter anyway. **Link to tracking Issue:** open-telemetry#34430 **Testing:** <Describe what testing was performed and which tests were added.> **Documentation:** <Describe the documentation added.> --------- Co-authored-by: Tyler Helmuth <[email protected]>
1 parent f95abf3 commit 1ac21d6

File tree

6 files changed

+79
-6
lines changed

6 files changed

+79
-6
lines changed

.chloggen/aggregate_labels_empty.yaml

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: metricstransform
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: The previously removed functionality of aggregating against an empty label set is restored.
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [34430]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: []

internal/coreinternal/aggregateutil/aggregate.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,21 @@ func CopyMetricDetails(from, to pmetric.Metric) {
3333
}
3434

3535
func FilterAttrs(metric pmetric.Metric, filterAttrKeys []string) {
36-
if len(filterAttrKeys) == 0 {
36+
// filterAttrKeys being nil means the filter is to be skipped.
37+
if filterAttrKeys == nil {
3738
return
3839
}
40+
// filterAttrKeys being empty means it is explicitly expected to filter
41+
// against an empty label set, which is functionally the same as removing
42+
// all attributes.
43+
if len(filterAttrKeys) == 0 {
44+
RangeDataPointAttributes(metric, func(attrs pcommon.Map) bool {
45+
attrs.Clear()
46+
return true
47+
})
48+
}
49+
// filterAttrKeys having provided attributes means the filter continues
50+
// as normal.
3951
RangeDataPointAttributes(metric, func(attrs pcommon.Map) bool {
4052
attrs.RemoveIf(func(k string, _ pcommon.Value) bool {
4153
return isNotPresent(k, filterAttrKeys)

internal/coreinternal/aggregateutil/aggregate_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,7 @@ func Test_FilterAttributes(t *testing.T) {
125125
want: func() pmetric.Metric {
126126
m := pmetric.NewMetric()
127127
s := m.SetEmptySum()
128-
d := s.DataPoints().AppendEmpty()
129-
d.Attributes().PutStr("attr1", "val1")
130-
d.Attributes().PutStr("attr2", "val2")
128+
s.DataPoints().AppendEmpty()
131129
return m
132130
},
133131
},

processor/metricstransformprocessor/metrics_testcase_builder_test.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
package metricstransformprocessor
55

66
import (
7+
"fmt"
8+
79
"go.opentelemetry.io/collector/pdata/pcommon"
810
"go.opentelemetry.io/collector/pdata/pmetric"
911
)
@@ -54,7 +56,12 @@ func (b builder) addDoubleDatapoint(start, ts pcommon.Timestamp, val float64, at
5456

5557
func (b builder) setAttrs(attrs pcommon.Map, attrValues []string) {
5658
if len(attrValues) != len(b.attrs) {
57-
panic(attrValues)
59+
panic(
60+
fmt.Sprintf(
61+
"not enough attributes, expected %d attributes but got %s",
62+
len(b.attrs),
63+
attrValues),
64+
)
5865
}
5966
for i, a := range b.attrs {
6067
attrs.PutStr(a, attrValues[i])

processor/metricstransformprocessor/metrics_transform_processor_otlp.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -554,7 +554,7 @@ func transformMetric(metric pmetric.Metric, transform internalTransform) bool {
554554
updateLabelOp(metric, op, transform.MetricIncludeFilter)
555555
case aggregateLabels:
556556
if canChangeMetric {
557-
attrs := []string{}
557+
attrs := make([]string, 0, len(op.labelSetMap))
558558
for k, v := range op.labelSetMap {
559559
if v {
560560
attrs = append(attrs, k)

processor/metricstransformprocessor/metrics_transform_processor_testcases_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -667,6 +667,35 @@ var (
667667
build(),
668668
},
669669
},
670+
{
671+
name: "metric_label_aggregation_empty_label_set",
672+
transforms: []internalTransform{
673+
{
674+
MetricIncludeFilter: internalFilterStrict{include: "metric1"},
675+
Action: Update,
676+
Operations: []internalOperation{
677+
{
678+
configOperation: Operation{
679+
Action: aggregateLabels,
680+
AggregationType: aggregateutil.Sum,
681+
LabelSet: []string{},
682+
},
683+
labelSetMap: map[string]bool{},
684+
},
685+
},
686+
},
687+
},
688+
in: []pmetric.Metric{
689+
metricBuilder(pmetric.MetricTypeGauge, "metric1", "label1", "label2", "label3").
690+
addIntDatapoint(0, 1, 1, "a", "b", "c").
691+
build(),
692+
},
693+
out: []pmetric.Metric{
694+
metricBuilder(pmetric.MetricTypeGauge, "metric1").
695+
addIntDatapoint(0, 1, 1).
696+
build(),
697+
},
698+
},
670699
{
671700
name: "metric_label_aggregation_ignored_for_partial_metric_match",
672701
transforms: []internalTransform{

0 commit comments

Comments
 (0)