[processor/tailsampling] fix the behavior of inverse numeric filters (o…

…pen-telemetry#34416) **Description:** This PR fixes the behaviour of numeric attribute filters with the `inverse_match` option set to `true`. In this case, the numeric filter now returns `InvertNotSampled`/`InvertSampled` if its condition matches, to make sure a span with matching attributes is not sampled even though other policies might yield a `Sampled` result. **Link to tracking Issue:** open-telemetry#34296 **Testing:** Added unit tests **Documentation:** No changes here, as the expected behavior is already described in the docs --------- Signed-off-by: Florian Bacher <[email protected]>
f7o · Sep 12, 2024 · 1ac0d79 · 1ac0d79
1 parent 0c95319
commit 1ac0d79
Show file tree

Hide file tree

Showing 3 changed files with 115 additions and 14 deletions.
diff --git a/.chloggen/fix_tail_sampling_processor_inverted_sampling.yaml b/.chloggen/fix_tail_sampling_processor_inverted_sampling.yaml
@@ -0,0 +1,27 @@
+# Use this changelog template to create an entry for release notes.
+
+# 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: tailsamplingprocessor
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Fix the behavior for numeric tag filters with `inverse_match` set to `true`.
+
+# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
+issues: [34296]
+
+# (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:
+
+# If your change doesn't affect end users or the exported elements of any package,
+# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: []
diff --git a/processor/tailsamplingprocessor/internal/sampling/numeric_tag_filter.go b/processor/tailsamplingprocessor/internal/sampling/numeric_tag_filter.go
@@ -39,14 +39,48 @@ func (naf *numericAttributeFilter) Evaluate(_ context.Context, _ pcommon.TraceID
 	defer trace.Unlock()
 	batches := trace.ReceivedBatches
 
-	return hasSpanWithCondition(batches, func(span ptrace.Span) bool {
-		if v, ok := span.Attributes().Get(naf.key); ok {
-			value := v.Int()
-			if value >= naf.minValue && value <= naf.maxValue {
-				return !(naf.invertMatch)
-
+	if naf.invertMatch {
+		return invertHasResourceOrSpanWithCondition(
+			batches,
+			func(resource pcommon.Resource) bool {
+				if v, ok := resource.Attributes().Get(naf.key); ok {
+					value := v.Int()
+					if value >= naf.minValue && value <= naf.maxValue {
+						return false
+					}
+				}
+				return true
+			},
+			func(span ptrace.Span) bool {
+				if v, ok := span.Attributes().Get(naf.key); ok {
+					value := v.Int()
+					if value >= naf.minValue && value <= naf.maxValue {
+						return false
+					}
+				}
+				return true
+			},
+		), nil
+	}
+	return hasResourceOrSpanWithCondition(
+		batches,
+		func(resource pcommon.Resource) bool {
+			if v, ok := resource.Attributes().Get(naf.key); ok {
+				value := v.Int()
+				if value >= naf.minValue && value <= naf.maxValue {
+					return true
+				}
+			}
+			return false
+		},
+		func(span ptrace.Span) bool {
+			if v, ok := span.Attributes().Get(naf.key); ok {
+				value := v.Int()
+				if value >= naf.minValue && value <= naf.maxValue {
+					return true
+				}
 			}
-		}
-		return naf.invertMatch
-	}), nil
+			return false
+		},
+	), nil
 }
diff --git a/processor/tailsamplingprocessor/internal/sampling/numeric_tag_filter_test.go b/processor/tailsamplingprocessor/internal/sampling/numeric_tag_filter_test.go
@@ -38,21 +38,41 @@ func TestNumericTagFilter(t *testing.T) {
 			Trace:    newTraceIntAttrs(empty, "example", math.MinInt32),
 			Decision: Sampled,
 		},
+		{
+			Desc:     "resource attribute at the lower limit",
+			Trace:    newTraceIntAttrs(map[string]any{"example": math.MinInt32}, "non_matching", math.MinInt32),
+			Decision: Sampled,
+		},
 		{
 			Desc:     "span attribute at the upper limit",
 			Trace:    newTraceIntAttrs(empty, "example", math.MaxInt32),
 			Decision: Sampled,
 		},
+		{
+			Desc:     "resource attribute at the upper limit",
+			Trace:    newTraceIntAttrs(map[string]any{"example": math.MaxInt32}, "non_matching", math.MaxInt),
+			Decision: Sampled,
+		},
 		{
 			Desc:     "span attribute below min limit",
 			Trace:    newTraceIntAttrs(empty, "example", math.MinInt32-1),
 			Decision: NotSampled,
 		},
+		{
+			Desc:     "resource attribute below min limit",
+			Trace:    newTraceIntAttrs(map[string]any{"example": math.MinInt32 - 1}, "non_matching", math.MinInt32),
+			Decision: NotSampled,
+		},
 		{
 			Desc:     "span attribute above max limit",
 			Trace:    newTraceIntAttrs(empty, "example", math.MaxInt32+1),
 			Decision: NotSampled,
 		},
+		{
+			Desc:     "resource attribute above max limit",
+			Trace:    newTraceIntAttrs(map[string]any{"example": math.MaxInt32 + 1}, "non_matching", math.MaxInt32),
+			Decision: NotSampled,
+		},
 	}
 
 	for _, c := range cases {
@@ -81,27 +101,47 @@ func TestNumericTagFilterInverted(t *testing.T) {
 		{
 			Desc:     "nonmatching span attribute",
 			Trace:    newTraceIntAttrs(empty, "non_matching", math.MinInt32),
-			Decision: Sampled,
+			Decision: InvertSampled,
 		},
 		{
 			Desc:     "span attribute at the lower limit",
 			Trace:    newTraceIntAttrs(empty, "example", math.MinInt32),
-			Decision: NotSampled,
+			Decision: InvertNotSampled,
+		},
+		{
+			Desc:     "resource attribute at the lower limit",
+			Trace:    newTraceIntAttrs(map[string]any{"example": math.MinInt32}, "non_matching", math.MinInt32),
+			Decision: InvertNotSampled,
 		},
 		{
 			Desc:     "span attribute at the upper limit",
 			Trace:    newTraceIntAttrs(empty, "example", math.MaxInt32),
-			Decision: NotSampled,
+			Decision: InvertNotSampled,
+		},
+		{
+			Desc:     "resource attribute at the upper limit",
+			Trace:    newTraceIntAttrs(map[string]any{"example": math.MaxInt32}, "non_matching", math.MaxInt32),
+			Decision: InvertNotSampled,
 		},
 		{
 			Desc:     "span attribute below min limit",
 			Trace:    newTraceIntAttrs(empty, "example", math.MinInt32-1),
-			Decision: Sampled,
+			Decision: InvertSampled,
+		},
+		{
+			Desc:     "resource attribute below min limit",
+			Trace:    newTraceIntAttrs(map[string]any{"example": math.MinInt32 - 1}, "non_matching", math.MinInt32),
+			Decision: InvertSampled,
 		},
 		{
 			Desc:     "span attribute above max limit",
 			Trace:    newTraceIntAttrs(empty, "example", math.MaxInt32+1),
-			Decision: Sampled,
+			Decision: InvertSampled,
+		},
+		{
+			Desc:     "resource attribute above max limit",
+			Trace:    newTraceIntAttrs(map[string]any{"example": math.MaxInt32 + 1}, "non_matching", math.MaxInt32+1),
+			Decision: InvertSampled,
 		},
 	}