Skip to content

Commit d1d4d69

Browse files
authored
[processor/tailsampling] allow setting only min_value or max_value for the numeric_attribute (#37328)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description This PR makes the `numeric_attribute` more flexible and allows to set only `min_value` or `max_value`, without the need to set both. This is useful to have simple configurations like these: ``` { type: numeric_attribute, numeric_attribute: { key: http.status_code, min_value: 400 } } ``` <!-- Issue number (e.g. #1234) or full URL to issue, if applicable. --> #### Link to tracking issue N/A <!--Describe what testing was performed and which tests were added.--> #### Testing Added unit tests to cover all changes and scenarios. <!--Describe the documentation added.--> #### Documentation I fixed an example on the documentation to have just the `min_value` set <!--Please delete paragraphs that you did not use before submitting.-->
1 parent 61d70f8 commit d1d4d69

File tree

6 files changed

+220
-20
lines changed

6 files changed

+220
-20
lines changed
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
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: enhancement
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: tailsamplingprocessor
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: makes the `numeric_attribute` more flexible and allows to set only `min_value` or `max_value`, without the need to set both
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: [37328]
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+
This is useful to have simple configurations like these:
20+
```
21+
{
22+
type: numeric_attribute,
23+
numeric_attribute: {
24+
key: http.status_code,
25+
min_value: 400
26+
}
27+
}
28+
```
29+
30+
31+
# If your change doesn't affect end users or the exported elements of any package,
32+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
33+
# Optional: The change log or logs in which this entry should be included.
34+
# e.g. '[user]' or '[user, api]'
35+
# Include 'user' if the change is relevant to end users.
36+
# Include 'api' if there is a change to a library API.
37+
# Default: '[user]'
38+
change_logs: [user]

processor/tailsamplingprocessor/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ processors:
186186
{
187187
name: test-composite-policy-1,
188188
type: numeric_attribute,
189-
numeric_attribute: {key: key1, min_value: 50, max_value: 100}
189+
numeric_attribute: {key: key1, min_value: 50}
190190
},
191191
{
192192
name: test-composite-policy-2,

processor/tailsamplingprocessor/internal/sampling/composite_test.go

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,10 @@ func newTraceWithKV(traceID pcommon.TraceID, key string, val int64) *TraceData {
5858

5959
func TestCompositeEvaluatorNotSampled(t *testing.T) {
6060
// Create 2 policies which do not match any trace
61-
n1 := NewNumericAttributeFilter(componenttest.NewNopTelemetrySettings(), "tag", 0, 100, false)
62-
n2 := NewNumericAttributeFilter(componenttest.NewNopTelemetrySettings(), "tag", 200, 300, false)
61+
min0 := int64(0)
62+
max100 := int64(100)
63+
n1 := NewNumericAttributeFilter(componenttest.NewNopTelemetrySettings(), "tag", &min0, &max100, false)
64+
n2 := NewNumericAttributeFilter(componenttest.NewNopTelemetrySettings(), "tag", &min0, &max100, false)
6365
c := NewComposite(zap.NewNop(), 1000, []SubPolicyEvalParams{{n1, 100}, {n2, 100}}, FakeTimeProvider{})
6466

6567
trace := createTrace()
@@ -75,7 +77,9 @@ func TestCompositeEvaluatorNotSampled(t *testing.T) {
7577

7678
func TestCompositeEvaluatorSampled(t *testing.T) {
7779
// Create 2 subpolicies. First results in 100% NotSampled, the second in 100% Sampled.
78-
n1 := NewNumericAttributeFilter(componenttest.NewNopTelemetrySettings(), "tag", 0, 100, false)
80+
min0 := int64(0)
81+
max100 := int64(100)
82+
n1 := NewNumericAttributeFilter(componenttest.NewNopTelemetrySettings(), "tag", &min0, &max100, false)
7983
n2 := NewAlwaysSample(componenttest.NewNopTelemetrySettings())
8084
c := NewComposite(zap.NewNop(), 1000, []SubPolicyEvalParams{{n1, 100}, {n2, 100}}, FakeTimeProvider{})
8185

@@ -93,7 +97,9 @@ func TestCompositeEvaluator_OverflowAlwaysSampled(t *testing.T) {
9397
timeProvider := &FakeTimeProvider{second: 0}
9498

9599
// Create 2 subpolicies. First results in 100% NotSampled, the second in 100% Sampled.
96-
n1 := NewNumericAttributeFilter(componenttest.NewNopTelemetrySettings(), "tag", 0, 100, false)
100+
min0 := int64(0)
101+
max100 := int64(100)
102+
n1 := NewNumericAttributeFilter(componenttest.NewNopTelemetrySettings(), "tag", &min0, &max100, false)
97103
n2 := NewAlwaysSample(componenttest.NewNopTelemetrySettings())
98104
c := NewComposite(zap.NewNop(), 3, []SubPolicyEvalParams{{n1, 1}, {n2, 1}}, timeProvider)
99105

@@ -126,7 +132,9 @@ func TestCompositeEvaluator_OverflowAlwaysSampled(t *testing.T) {
126132

127133
func TestCompositeEvaluatorSampled_AlwaysSampled(t *testing.T) {
128134
// Create 2 subpolicies. First results in 100% NotSampled, the second in 100% Sampled.
129-
n1 := NewNumericAttributeFilter(componenttest.NewNopTelemetrySettings(), "tag", 0, 100, false)
135+
min0 := int64(0)
136+
max100 := int64(100)
137+
n1 := NewNumericAttributeFilter(componenttest.NewNopTelemetrySettings(), "tag", &min0, &max100, false)
130138
n2 := NewAlwaysSample(componenttest.NewNopTelemetrySettings())
131139
c := NewComposite(zap.NewNop(), 10, []SubPolicyEvalParams{{n1, 20}, {n2, 20}}, FakeTimeProvider{})
132140

@@ -201,7 +209,9 @@ func TestCompositeEvaluatorThrottling(t *testing.T) {
201209
}
202210

203211
func TestCompositeEvaluator2SubpolicyThrottling(t *testing.T) {
204-
n1 := NewNumericAttributeFilter(componenttest.NewNopTelemetrySettings(), "tag", 0, 100, false)
212+
min0 := int64(0)
213+
max100 := int64(100)
214+
n1 := NewNumericAttributeFilter(componenttest.NewNopTelemetrySettings(), "tag", &min0, &max100, false)
205215
n2 := NewAlwaysSample(componenttest.NewNopTelemetrySettings())
206216
timeProvider := &FakeTimeProvider{second: 0}
207217
const totalSPS = 10
@@ -262,3 +272,26 @@ func TestCompositeEvaluator2SubpolicyThrottling(t *testing.T) {
262272
assert.Equal(t, expected, decision)
263273
}
264274
}
275+
276+
func TestComposite(t *testing.T) {
277+
// Define the int64 values
278+
min0 := int64(0)
279+
max100 := int64(100)
280+
281+
// Update the policy creation calls
282+
filter1 := NewNumericAttributeFilter(componenttest.NewNopTelemetrySettings(), "tag", &min0, &max100, false)
283+
284+
// Create 2 subpolicies. First results in 100% NotSampled, the second in 100% Sampled.
285+
n1 := filter1
286+
n2 := NewAlwaysSample(componenttest.NewNopTelemetrySettings())
287+
c := NewComposite(zap.NewNop(), 1000, []SubPolicyEvalParams{{n1, 100}, {n2, 100}}, FakeTimeProvider{})
288+
289+
trace := createTrace()
290+
291+
decision, err := c.Evaluate(context.Background(), traceID, trace)
292+
require.NoError(t, err, "Failed to evaluate composite policy: %v", err)
293+
294+
// The second policy is AlwaysSample, so the decision should be Sampled.
295+
expected := Sampled
296+
assert.Equal(t, expected, decision)
297+
}

processor/tailsamplingprocessor/internal/sampling/numeric_tag_filter.go

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package sampling // import "github.com/open-telemetry/opentelemetry-collector-co
55

66
import (
77
"context"
8+
"math"
89

910
"go.opentelemetry.io/collector/component"
1011
"go.opentelemetry.io/collector/pdata/pcommon"
@@ -13,17 +14,23 @@ import (
1314
)
1415

1516
type numericAttributeFilter struct {
16-
key string
17-
minValue, maxValue int64
18-
logger *zap.Logger
19-
invertMatch bool
17+
key string
18+
minValue *int64
19+
maxValue *int64
20+
logger *zap.Logger
21+
invertMatch bool
2022
}
2123

2224
var _ PolicyEvaluator = (*numericAttributeFilter)(nil)
2325

2426
// NewNumericAttributeFilter creates a policy evaluator that samples all traces with
25-
// the given attribute in the given numeric range.
26-
func NewNumericAttributeFilter(settings component.TelemetrySettings, key string, minValue, maxValue int64, invertMatch bool) PolicyEvaluator {
27+
// the given attribute in the given numeric range. If minValue is nil, it will use math.MinInt64.
28+
// If maxValue is nil, it will use math.MaxInt64. At least one of minValue or maxValue must be set.
29+
func NewNumericAttributeFilter(settings component.TelemetrySettings, key string, minValue, maxValue *int64, invertMatch bool) PolicyEvaluator {
30+
if minValue == nil && maxValue == nil {
31+
settings.Logger.Error("At least one of minValue or maxValue must be set")
32+
return nil
33+
}
2734
return &numericAttributeFilter{
2835
key: key,
2936
minValue: minValue,
@@ -39,13 +46,23 @@ func (naf *numericAttributeFilter) Evaluate(_ context.Context, _ pcommon.TraceID
3946
defer trace.Unlock()
4047
batches := trace.ReceivedBatches
4148

49+
// Get the effective min/max values
50+
minVal := int64(math.MinInt64)
51+
if naf.minValue != nil {
52+
minVal = *naf.minValue
53+
}
54+
maxVal := int64(math.MaxInt64)
55+
if naf.maxValue != nil {
56+
maxVal = *naf.maxValue
57+
}
58+
4259
if naf.invertMatch {
4360
return invertHasResourceOrSpanWithCondition(
4461
batches,
4562
func(resource pcommon.Resource) bool {
4663
if v, ok := resource.Attributes().Get(naf.key); ok {
4764
value := v.Int()
48-
if value >= naf.minValue && value <= naf.maxValue {
65+
if value >= minVal && value <= maxVal {
4966
return false
5067
}
5168
}
@@ -54,7 +71,7 @@ func (naf *numericAttributeFilter) Evaluate(_ context.Context, _ pcommon.TraceID
5471
func(span ptrace.Span) bool {
5572
if v, ok := span.Attributes().Get(naf.key); ok {
5673
value := v.Int()
57-
if value >= naf.minValue && value <= naf.maxValue {
74+
if value >= minVal && value <= maxVal {
5875
return false
5976
}
6077
}
@@ -67,7 +84,7 @@ func (naf *numericAttributeFilter) Evaluate(_ context.Context, _ pcommon.TraceID
6784
func(resource pcommon.Resource) bool {
6885
if v, ok := resource.Attributes().Get(naf.key); ok {
6986
value := v.Int()
70-
if value >= naf.minValue && value <= naf.maxValue {
87+
if value >= minVal && value <= maxVal {
7188
return true
7289
}
7390
}
@@ -76,7 +93,7 @@ func (naf *numericAttributeFilter) Evaluate(_ context.Context, _ pcommon.TraceID
7693
func(span ptrace.Span) bool {
7794
if v, ok := span.Attributes().Get(naf.key); ok {
7895
value := v.Int()
79-
if value >= naf.minValue && value <= naf.maxValue {
96+
if value >= minVal && value <= maxVal {
8097
return true
8198
}
8299
}

processor/tailsamplingprocessor/internal/sampling/numeric_tag_filter_test.go

Lines changed: 112 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,17 @@ import (
1010

1111
"github.com/google/uuid"
1212
"github.com/stretchr/testify/assert"
13+
"github.com/stretchr/testify/require"
1314
"go.opentelemetry.io/collector/component/componenttest"
1415
"go.opentelemetry.io/collector/pdata/pcommon"
1516
"go.opentelemetry.io/collector/pdata/ptrace"
1617
)
1718

1819
func TestNumericTagFilter(t *testing.T) {
1920
empty := map[string]any{}
20-
filter := NewNumericAttributeFilter(componenttest.NewNopTelemetrySettings(), "example", math.MinInt32, math.MaxInt32, false)
21+
minVal := int64(math.MinInt32)
22+
maxVal := int64(math.MaxInt32)
23+
filter := NewNumericAttributeFilter(componenttest.NewNopTelemetrySettings(), "example", &minVal, &maxVal, false)
2124

2225
resAttr := map[string]any{}
2326
resAttr["example"] = 8
@@ -86,7 +89,9 @@ func TestNumericTagFilter(t *testing.T) {
8689

8790
func TestNumericTagFilterInverted(t *testing.T) {
8891
empty := map[string]any{}
89-
filter := NewNumericAttributeFilter(componenttest.NewNopTelemetrySettings(), "example", math.MinInt32, math.MaxInt32, true)
92+
minVal := int64(math.MinInt32)
93+
maxVal := int64(math.MaxInt32)
94+
filter := NewNumericAttributeFilter(componenttest.NewNopTelemetrySettings(), "example", &minVal, &maxVal, true)
9095

9196
resAttr := map[string]any{}
9297
resAttr["example"] = 8
@@ -153,6 +158,111 @@ func TestNumericTagFilterInverted(t *testing.T) {
153158
}
154159
}
155160

161+
func TestNumericTagFilterOptionalBounds(t *testing.T) {
162+
tests := []struct {
163+
name string
164+
min *int64
165+
max *int64
166+
value int64
167+
invertMatch bool
168+
want Decision
169+
}{
170+
{
171+
name: "only min set - value above min",
172+
min: ptr(int64(100)),
173+
max: nil,
174+
value: 200,
175+
want: Sampled,
176+
},
177+
{
178+
name: "only min set - value below min",
179+
min: ptr(int64(100)),
180+
max: nil,
181+
value: 50,
182+
want: NotSampled,
183+
},
184+
{
185+
name: "only max set - value below max",
186+
min: nil,
187+
max: ptr(int64(100)),
188+
value: 50,
189+
want: Sampled,
190+
},
191+
{
192+
name: "only max set - value above max",
193+
min: nil,
194+
max: ptr(int64(100)),
195+
value: 200,
196+
want: NotSampled,
197+
},
198+
{
199+
name: "both set - value in range",
200+
min: ptr(int64(100)),
201+
max: ptr(int64(200)),
202+
value: 150,
203+
want: Sampled,
204+
},
205+
{
206+
name: "both set - value out of range",
207+
min: ptr(int64(100)),
208+
max: ptr(int64(200)),
209+
value: 50,
210+
want: NotSampled,
211+
},
212+
{
213+
name: "inverted match - only min set - value above min",
214+
min: ptr(int64(100)),
215+
max: nil,
216+
value: 200,
217+
invertMatch: true,
218+
want: InvertNotSampled,
219+
},
220+
{
221+
name: "inverted match - only max set - value below max",
222+
min: nil,
223+
max: ptr(int64(100)),
224+
value: 50,
225+
invertMatch: true,
226+
want: InvertNotSampled,
227+
},
228+
}
229+
230+
for _, tt := range tests {
231+
t.Run(tt.name, func(t *testing.T) {
232+
filter := NewNumericAttributeFilter(componenttest.NewNopTelemetrySettings(), "example", tt.min, tt.max, tt.invertMatch)
233+
require.NotNil(t, filter, "filter should not be nil")
234+
235+
trace := newTraceIntAttrs(map[string]any{}, "example", tt.value)
236+
decision, err := filter.Evaluate(context.Background(), pcommon.TraceID{}, trace)
237+
assert.NoError(t, err)
238+
assert.Equal(t, tt.want, decision)
239+
})
240+
}
241+
}
242+
243+
func TestNumericTagFilterNilBounds(t *testing.T) {
244+
settings := componenttest.NewNopTelemetrySettings()
245+
filter := NewNumericAttributeFilter(settings, "example", nil, nil, false)
246+
assert.Nil(t, filter, "filter should be nil when both bounds are nil")
247+
248+
// Test that the filter is created successfully when at least one bound is set
249+
minBound := int64(100)
250+
filter = NewNumericAttributeFilter(settings, "example", &minBound, nil, false)
251+
assert.NotNil(t, filter, "filter should not be nil when min is set")
252+
253+
maxBound := int64(200)
254+
filter = NewNumericAttributeFilter(settings, "example", nil, &maxBound, false)
255+
assert.NotNil(t, filter, "filter should not be nil when max is set")
256+
257+
filter = NewNumericAttributeFilter(settings, "example", &minBound, &maxBound, false)
258+
assert.NotNil(t, filter, "filter should not be nil when both bounds are set")
259+
}
260+
261+
// helper function to create int64 pointer
262+
func ptr(i int64) *int64 {
263+
return &i
264+
}
265+
156266
func newTraceIntAttrs(nodeAttrs map[string]any, spanAttrKey string, spanAttrValue int64) *TraceData {
157267
traces := ptrace.NewTraces()
158268
rs := traces.ResourceSpans().AppendEmpty()

processor/tailsamplingprocessor/processor.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,9 @@ func getSharedPolicyEvaluator(settings component.TelemetrySettings, cfg *sharedP
210210
return sampling.NewLatency(settings, lfCfg.ThresholdMs, lfCfg.UpperThresholdmsMs), nil
211211
case NumericAttribute:
212212
nafCfg := cfg.NumericAttributeCfg
213-
return sampling.NewNumericAttributeFilter(settings, nafCfg.Key, nafCfg.MinValue, nafCfg.MaxValue, nafCfg.InvertMatch), nil
213+
minValue := nafCfg.MinValue
214+
maxValue := nafCfg.MaxValue
215+
return sampling.NewNumericAttributeFilter(settings, nafCfg.Key, &minValue, &maxValue, nafCfg.InvertMatch), nil
214216
case Probabilistic:
215217
pCfg := cfg.ProbabilisticCfg
216218
return sampling.NewProbabilisticSampler(settings, pCfg.HashSalt, pCfg.SamplingPercentage), nil

0 commit comments

Comments
 (0)