[processor/tailsampling] Deprecate invert sample decisions

.chloggen/tailsamplingprocessor-deprecate-invert-decisions.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: deprecation
+
+# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
+component: processor/tailsampling
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: The invert decisions (InvertSampled and InvertNotSampled) have been deprecated, please make use of drop policy to explicitly not sample select traces.
+
+# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
+issues: [39833]
+
+# (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: []

processor/tailsamplingprocessor/README.md

@@ -65,12 +65,12 @@ The following configuration options can also be modified:
 Each policy will result in a decision, and the processor will evaluate them to make a final decision:
 
 - When there's a "drop" decision, the trace is not sampled;
-- When there's an "inverted not sample" decision, the trace is not sampled;
+- When there's an "inverted not sample" decision, the trace is not sampled; ***Deprecated***
 - When there's a "sample" decision, the trace is sampled;
-- When there's a "inverted sample" decision and no "not sample" decisions, the trace is sampled;
+- When there's a "inverted sample" decision and no "not sample" decisions, the trace is sampled; ***Deprecated***
 - In all other cases, the trace is NOT sampled
 
-An "inverted" decision is the one made based on the "invert_match" attribute, such as the one from the string, numeric or boolean tag policy.
+An "inverted" decision is the one made based on the "invert_match" attribute, such as the one from the string, numeric or boolean tag policy. There is an exception to this if the policy is within an and or composite policy, the resulting decision will be either sampled or not sampled. The "inverted" decisions have been deprecated, please make use of drop policy to explicitly not sample select traces.
 
 Examples:
 
@@ -560,6 +560,12 @@ When this feature gate is set, this will add additional attributes on each sampl
 | `tailsampling.policy`           | Records the configured name of the policy that sampled a trace            | Always                     |
 | `tailsampling.composite_policy` | Records the configured name of a composite subpolicy that sampled a trace | When composite policy used |
 
+### Disable invert decisions
+
+The invert sampling decisions (`InvertSampled` and `InvertNotSampled`) have been deprecated, however, they are still available. To disable them before their complete removal, you can use the `processor.tailsamplingprocessor.disableinvertdecisions` feature gate. When this feature gate is set, sampling policy `invert_match` will result in a `Sampled` or `NotSampled` decision instead of `InvertSampled` or `InvertNotSampled`. This applies to the string, numeric, and boolean tag policy.
+
+If you disable invert decisions, you can make use of drop policy to explicitly not sample select traces.
+
 ### Policy Evaluation Errors
 
 ```

processor/tailsamplingprocessor/internal/sampling/boolean_tag_filter_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/assert"
 	"go.opentelemetry.io/collector/component/componenttest"
+	"go.opentelemetry.io/collector/featuregate"
 	"go.opentelemetry.io/collector/pdata/pcommon"
 	"go.opentelemetry.io/collector/pdata/ptrace"
 )
@@ -61,9 +62,10 @@ func TestBooleanTagFilterInverted(t *testing.T) {
 	resAttr["example"] = 8
 
 	cases := []struct {
-		Desc     string
-		Trace    *TraceData
-		Decision Decision
+		Desc                  string
+		Trace                 *TraceData
+		Decision              Decision
+		DisableInvertDecision bool
 	}{
 		{
 			Desc:     "non-matching span attribute",
@@ -80,10 +82,30 @@ func TestBooleanTagFilterInverted(t *testing.T) {
 			Trace:    newTraceBoolAttrs(empty, "example", true),
 			Decision: InvertNotSampled,
 		},
+		{
+			Desc:                  "span attribute with non matching boolean value with DisableInvertDecision",
+			Trace:                 newTraceBoolAttrs(empty, "example", false),
+			Decision:              Sampled,
+			DisableInvertDecision: true,
+		},
+		{
+			Desc:                  "span attribute with matching boolean value with DisableInvertDecision",
+			Trace:                 newTraceBoolAttrs(empty, "example", true),
+			Decision:              NotSampled,
+			DisableInvertDecision: true,
+		},
 	}
 
 	for _, c := range cases {
 		t.Run(c.Desc, func(t *testing.T) {
+			if c.DisableInvertDecision {
+				err := featuregate.GlobalRegistry().Set("processor.tailsamplingprocessor.disableinvertdecisions", true)
+				assert.NoError(t, err)
+				defer func() {
+					err := featuregate.GlobalRegistry().Set("processor.tailsamplingprocessor.disableinvertdecisions", false)
+					assert.NoError(t, err)
+				}()
+			}
 			u, _ := uuid.NewRandom()
 			decision, err := filter.Evaluate(context.Background(), pcommon.TraceID(u), c.Trace)
 			assert.NoError(t, err)

processor/tailsamplingprocessor/internal/sampling/featureflag.go

@@ -0,0 +1,16 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package sampling // import "github.com/open-telemetry/opentelemetry-collector-contrib/processor/tailsamplingprocessor/internal/sampling"
+
+import "go.opentelemetry.io/collector/featuregate"
+
+var disableInvertDecisions = featuregate.GlobalRegistry().MustRegister(
+	"processor.tailsamplingprocessor.disableinvertdecisions",
+	featuregate.StageAlpha,
+	featuregate.WithRegisterDescription("When enabled, sampling policy 'invert_match' will result in a SAMPLED or NOT SAMPLED decision instead of INVERT SAMPLED or INVERT NOT SAMPLED."),
+)
+
+func IsInvertDecisionsDisabled() bool {
+	return disableInvertDecisions.IsEnabled()
+}

processor/tailsamplingprocessor/internal/sampling/numeric_tag_filter_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.opentelemetry.io/collector/component/componenttest"
+	"go.opentelemetry.io/collector/featuregate"
 	"go.opentelemetry.io/collector/pdata/pcommon"
 	"go.opentelemetry.io/collector/pdata/ptrace"
 )
@@ -97,9 +98,10 @@ func TestNumericTagFilterInverted(t *testing.T) {
 	resAttr["example"] = 8
 
 	cases := []struct {
-		Desc     string
-		Trace    *TraceData
-		Decision Decision
+		Desc                  string
+		Trace                 *TraceData
+		Decision              Decision
+		DisableInvertDecision bool
 	}{
 		{
 			Desc:     "nonmatching span attribute",
@@ -146,10 +148,30 @@ func TestNumericTagFilterInverted(t *testing.T) {
 			Trace:    newTraceIntAttrs(map[string]any{"example": math.MaxInt32 + 1}, "non_matching", math.MaxInt32+1),
 			Decision: InvertSampled,
 		},
+		{
+			Desc:                  "nonmatching span attribute with DisableInvertDecision",
+			Trace:                 newTraceIntAttrs(empty, "non_matching", math.MinInt32),
+			Decision:              Sampled,
+			DisableInvertDecision: true,
+		},
+		{
+			Desc:                  "span attribute at the lower limit with DisableInvertDecision",
+			Trace:                 newTraceIntAttrs(empty, "example", math.MinInt32),
+			Decision:              NotSampled,
+			DisableInvertDecision: true,
+		},
 	}
 
 	for _, c := range cases {
 		t.Run(c.Desc, func(t *testing.T) {
+			if c.DisableInvertDecision {
+				err := featuregate.GlobalRegistry().Set("processor.tailsamplingprocessor.disableinvertdecisions", true)
+				assert.NoError(t, err)
+				defer func() {
+					err := featuregate.GlobalRegistry().Set("processor.tailsamplingprocessor.disableinvertdecisions", false)
+					assert.NoError(t, err)
+				}()
+			}
 			u, _ := uuid.NewRandom()
 			decision, err := filter.Evaluate(context.Background(), pcommon.TraceID(u), c.Trace)
 			assert.NoError(t, err)

processor/tailsamplingprocessor/internal/sampling/string_tag_filter_test.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"go.opentelemetry.io/collector/component/componenttest"
+	"go.opentelemetry.io/collector/featuregate"
 	"go.opentelemetry.io/collector/pdata/pcommon"
 	"go.opentelemetry.io/collector/pdata/ptrace"
 )
@@ -24,10 +25,11 @@ type TestStringAttributeCfg struct {
 
 func TestStringTagFilter(t *testing.T) {
 	cases := []struct {
-		Desc      string
-		Trace     *TraceData
-		filterCfg *TestStringAttributeCfg
-		Decision  Decision
+		Desc                  string
+		Trace                 *TraceData
+		filterCfg             *TestStringAttributeCfg
+		Decision              Decision
+		DisableInvertDecision bool
 	}{
 		{
 			Desc:      "nonmatching node attribute key",
@@ -197,10 +199,32 @@ func TestStringTagFilter(t *testing.T) {
 			filterCfg: &TestStringAttributeCfg{Key: "example", Values: []string{}, EnabledRegexMatching: true, InvertMatch: true},
 			Decision:  InvertSampled,
 		},
+		{
+			Desc:                  "invert matching node attribute key with DisableInvertDecision",
+			Trace:                 newTraceStringAttrs(map[string]any{"example": "value"}, "", ""),
+			filterCfg:             &TestStringAttributeCfg{Key: "example", Values: []string{"value"}, EnabledRegexMatching: false, CacheMaxSize: defaultCacheSize, InvertMatch: true},
+			Decision:              NotSampled,
+			DisableInvertDecision: true,
+		},
+		{
+			Desc:                  "invert nonmatching node attribute key with DisableInvertDecision",
+			Trace:                 newTraceStringAttrs(map[string]any{"non_matching": "value"}, "", ""),
+			filterCfg:             &TestStringAttributeCfg{Key: "example", Values: []string{"value"}, EnabledRegexMatching: false, CacheMaxSize: defaultCacheSize, InvertMatch: true},
+			Decision:              Sampled,
+			DisableInvertDecision: true,
+		},
 	}
 
 	for _, c := range cases {
 		t.Run(c.Desc, func(t *testing.T) {
+			if c.DisableInvertDecision {
+				err := featuregate.GlobalRegistry().Set("processor.tailsamplingprocessor.disableinvertdecisions", true)
+				assert.NoError(t, err)
+				defer func() {
+					err := featuregate.GlobalRegistry().Set("processor.tailsamplingprocessor.disableinvertdecisions", false)
+					assert.NoError(t, err)
+				}()
+			}
 			filter := NewStringAttributeFilter(componenttest.NewNopTelemetrySettings(), c.filterCfg.Key, c.filterCfg.Values, c.filterCfg.EnabledRegexMatching, c.filterCfg.CacheMaxSize, c.filterCfg.InvertMatch)
 			decision, err := filter.Evaluate(context.Background(), pcommon.TraceID([16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}), c.Trace)
 			assert.NoError(t, err)

processor/tailsamplingprocessor/internal/sampling/util.go

@@ -23,7 +23,7 @@ func hasResourceOrSpanWithCondition(
 			return Sampled
 		}
 
-		if hasInstrumentationLibrarySpanWithCondition(rs.ScopeSpans(), shouldSampleSpan) {
+		if hasInstrumentationLibrarySpanWithCondition(rs.ScopeSpans(), shouldSampleSpan, false) {
 			return Sampled
 		}
 	}
@@ -37,18 +37,30 @@ func invertHasResourceOrSpanWithCondition(
 	shouldSampleResource func(resource pcommon.Resource) bool,
 	shouldSampleSpan func(span ptrace.Span) bool,
 ) Decision {
+	isd := IsInvertDecisionsDisabled()
+
 	for i := 0; i < td.ResourceSpans().Len(); i++ {
 		rs := td.ResourceSpans().At(i)
 
 		resource := rs.Resource()
 		if !shouldSampleResource(resource) {
+			if isd {
+				return NotSampled
+			}
 			return InvertNotSampled
 		}
 
-		if !invertHasInstrumentationLibrarySpanWithCondition(rs.ScopeSpans(), shouldSampleSpan) {
+		if !hasInstrumentationLibrarySpanWithCondition(rs.ScopeSpans(), shouldSampleSpan, true) {
+			if isd {
+				return NotSampled
+			}
 			return InvertNotSampled
 		}
 	}
+
+	if isd {
+		return Sampled
+	}
 	return InvertSampled
 }
 
@@ -57,41 +69,26 @@ func hasSpanWithCondition(td ptrace.Traces, shouldSample func(span ptrace.Span)
 	for i := 0; i < td.ResourceSpans().Len(); i++ {
 		rs := td.ResourceSpans().At(i)
 
-		if hasInstrumentationLibrarySpanWithCondition(rs.ScopeSpans(), shouldSample) {
+		if hasInstrumentationLibrarySpanWithCondition(rs.ScopeSpans(), shouldSample, false) {
 			return Sampled
 		}
 	}
 	return NotSampled
 }
 
-func hasInstrumentationLibrarySpanWithCondition(ilss ptrace.ScopeSpansSlice, check func(span ptrace.Span) bool) bool {
-	for i := 0; i < ilss.Len(); i++ {
-		ils := ilss.At(i)
-
-		for j := 0; j < ils.Spans().Len(); j++ {
-			span := ils.Spans().At(j)
-
-			if check(span) {
-				return true
-			}
-		}
-	}
-	return false
-}
-
-func invertHasInstrumentationLibrarySpanWithCondition(ilss ptrace.ScopeSpansSlice, check func(span ptrace.Span) bool) bool {
+func hasInstrumentationLibrarySpanWithCondition(ilss ptrace.ScopeSpansSlice, check func(span ptrace.Span) bool, invert bool) bool {
 	for i := 0; i < ilss.Len(); i++ {
 		ils := ilss.At(i)
 
 		for j := 0; j < ils.Spans().Len(); j++ {
 			span := ils.Spans().At(j)
 
-			if !check(span) {
-				return false
+			if r := check(span); r != invert {
+				return r
 			}
 		}
 	}
-	return true
+	return invert
 }
 
 func SetAttrOnScopeSpans(data *TraceData, attrName string, attrKey string) {