From 7ab1ac2e47074112e96391813b0542b73ec0b2bc Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Fri, 15 Aug 2025 14:09:42 +0900 Subject: [PATCH 1/4] enrichments: implement t-value representative count --- enrichments/internal/elastic/span.go | 39 ++++++++-- enrichments/internal/elastic/span_test.go | 94 ++++++++++++++++++++++- go.mod | 1 + go.sum | 2 + 4 files changed, 128 insertions(+), 8 deletions(-) diff --git a/enrichments/internal/elastic/span.go b/enrichments/internal/elastic/span.go index 5c677d7..b4da46b 100644 --- a/enrichments/internal/elastic/span.go +++ b/enrichments/internal/elastic/span.go @@ -32,6 +32,7 @@ import ( "github.com/elastic/opentelemetry-lib/elasticattr" "github.com/elastic/opentelemetry-lib/enrichments/config" + "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/sampling" "github.com/ua-parser/uap-go/uaparser" "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/ptrace" @@ -41,6 +42,11 @@ import ( "google.golang.org/grpc/codes" ) +// defaultRepresentativeCount is the representative count to use for adjusting +// sampled spans when a value could not be found from tracestate. Our default is +// to assume no sampling. +const defaultRepresentativeCount = 1.0 + // EnrichSpan adds Elastic specific attributes to the OTel span. // These attributes are derived from the base attributes and appended to // the span attributes. The enrichment logic is performed by categorizing @@ -658,20 +664,39 @@ func (s *spanEventEnrichmentContext) enrich( // with a difference that representative count can also include // dynamically calculated representivity for non-probabilistic sampling. // In addition, the representative count defaults to 1 if the adjusted -// count is UNKNOWN or the p-value is invalid. +// count is UNKNOWN or the th-value is invalid. +// +// Def: https://opentelemetry.io/docs/specs/otel/trace/tracestate-probability-sampling/#converting-threshold-to-an-adjusted-count-sampling-rate +// +// The count is calculated by using t-value: +// https://opentelemetry.io/docs/specs/otel/trace/tracestate-probability-sampling/#rejection-threshold-t +// +// For compatibility, older calculation is also supported. // // Def: https://opentelemetry.io/docs/specs/otel/trace/tracestate-probability-sampling/#adjusted-count) // // The count is calculated by using p-value: // https://opentelemetry.io/docs/reference/specification/trace/tracestate-probability-sampling/#p-value func getRepresentativeCount(tracestate string) float64 { - var p uint64 - otValue := getValueForKeyInString(tracestate, "ot", ',', '=') - if otValue != "" { - pValue := getValueForKeyInString(otValue, "p", ';', ':') + w3cts, err := sampling.NewW3CTraceState(tracestate) + if err != nil || w3cts.OTelValue() == nil { + return defaultRepresentativeCount + } - if pValue != "" { - p, _ = strconv.ParseUint(pValue, 10, 6) + otts := w3cts.OTelValue() + // Use t-value if provided. + if th, ok := otts.TValueThreshold(); ok { + // Small optimization for always-sampled case since it's commonly used. + if th.Unsigned() == 0 { + return defaultRepresentativeCount + } + return th.AdjustedCount() + } + + var p uint64 + for _, kv := range otts.ExtraValues() { + if kv.Key == "p" && kv.Value != "" { + p, _ = strconv.ParseUint(kv.Value, 10, 6) } } diff --git a/enrichments/internal/elastic/span_test.go b/enrichments/internal/elastic/span_test.go index f11b27b..7e37d7b 100644 --- a/enrichments/internal/elastic/span_test.go +++ b/enrichments/internal/elastic/span_test.go @@ -92,7 +92,7 @@ func TestElasticTransactionEnrich(t *testing.T) { name: "with_pvalue", input: func() ptrace.Span { span := ptrace.NewSpan() - span.TraceState().FromRaw("ot=p:8;") + span.TraceState().FromRaw("ot=p:8") return span }(), config: config.Enabled().Transaction, @@ -111,6 +111,98 @@ func TestElasticTransactionEnrich(t *testing.T) { elasticattr.TransactionType: "unknown", }, }, + { + name: "with_tvalue (ratio 1.0)", + input: func() ptrace.Span { + span := ptrace.NewSpan() + span.TraceState().FromRaw("ot=th:0") + return span + }(), + config: config.Enabled().Transaction, + enrichedAttrs: map[string]any{ + elasticattr.TimestampUs: int64(0), + elasticattr.TransactionSampled: true, + elasticattr.TransactionRoot: true, + elasticattr.TransactionID: "", + elasticattr.TransactionName: "", + elasticattr.ProcessorEvent: "transaction", + elasticattr.TransactionRepresentativeCount: float64(1), + elasticattr.TransactionDurationUs: int64(0), + elasticattr.EventOutcome: "success", + elasticattr.SuccessCount: int64(1), + elasticattr.TransactionResult: "Success", + elasticattr.TransactionType: "unknown", + }, + }, + { + name: "with_tvalue (ratio 0.5)", + input: func() ptrace.Span { + span := ptrace.NewSpan() + span.TraceState().FromRaw("ot=th:8") + return span + }(), + config: config.Enabled().Transaction, + enrichedAttrs: map[string]any{ + elasticattr.TimestampUs: int64(0), + elasticattr.TransactionSampled: true, + elasticattr.TransactionRoot: true, + elasticattr.TransactionID: "", + elasticattr.TransactionName: "", + elasticattr.ProcessorEvent: "transaction", + elasticattr.TransactionRepresentativeCount: float64(2), + elasticattr.TransactionDurationUs: int64(0), + elasticattr.EventOutcome: "success", + elasticattr.SuccessCount: int64(2), + elasticattr.TransactionResult: "Success", + elasticattr.TransactionType: "unknown", + }, + }, + { + name: "with_tvalue (ratio 0.25)", + input: func() ptrace.Span { + span := ptrace.NewSpan() + span.TraceState().FromRaw("ot=th:c") + return span + }(), + config: config.Enabled().Transaction, + enrichedAttrs: map[string]any{ + elasticattr.TimestampUs: int64(0), + elasticattr.TransactionSampled: true, + elasticattr.TransactionRoot: true, + elasticattr.TransactionID: "", + elasticattr.TransactionName: "", + elasticattr.ProcessorEvent: "transaction", + elasticattr.TransactionRepresentativeCount: float64(4), + elasticattr.TransactionDurationUs: int64(0), + elasticattr.EventOutcome: "success", + elasticattr.SuccessCount: int64(4), + elasticattr.TransactionResult: "Success", + elasticattr.TransactionType: "unknown", + }, + }, + { + name: "with_tvalue and p", + input: func() ptrace.Span { + span := ptrace.NewSpan() + span.TraceState().FromRaw("ot=th:c;p:8") + return span + }(), + config: config.Enabled().Transaction, + enrichedAttrs: map[string]any{ + elasticattr.TimestampUs: int64(0), + elasticattr.TransactionSampled: true, + elasticattr.TransactionRoot: true, + elasticattr.TransactionID: "", + elasticattr.TransactionName: "", + elasticattr.ProcessorEvent: "transaction", + elasticattr.TransactionRepresentativeCount: float64(4), + elasticattr.TransactionDurationUs: int64(0), + elasticattr.EventOutcome: "success", + elasticattr.SuccessCount: int64(4), + elasticattr.TransactionResult: "Success", + elasticattr.TransactionType: "unknown", + }, + }, { name: "http_status_ok", input: func() ptrace.Span { diff --git a/go.mod b/go.mod index 5132c57..44bd36a 100644 --- a/go.mod +++ b/go.mod @@ -10,6 +10,7 @@ require ( github.com/google/go-cmp v0.7.0 github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden v0.131.0 github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest v0.131.0 + github.com/open-telemetry/opentelemetry-collector-contrib/pkg/sampling v0.131.0 github.com/stretchr/testify v1.10.0 github.com/ua-parser/uap-go v0.0.0-20250213224047-9c035f085b90 go.opentelemetry.io/collector/component v1.37.0 diff --git a/go.sum b/go.sum index b442875..f3cdcee 100644 --- a/go.sum +++ b/go.sum @@ -77,6 +77,8 @@ github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest v0.131.0 github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest v0.131.0/go.mod h1:29C3uJZWtRu6wM4xqm023BcN6SpYge2eJs/5bAq2dGU= github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil v0.131.0 h1:lb0PuWuVEQPafzYBhrS8AX0sugKjG7RDi9+DRStjJhE= github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil v0.131.0/go.mod h1:PmUOI9ZjJpUXFp8V6NE/r0mnmJAmHU/x9A+N7YYMJ1o= +github.com/open-telemetry/opentelemetry-collector-contrib/pkg/sampling v0.131.0 h1:5sPZErUo231/wBoKp/HiEieW8uhiQXHIHnII3HXIcYM= +github.com/open-telemetry/opentelemetry-collector-contrib/pkg/sampling v0.131.0/go.mod h1:0eTDOk4+HDSo+3oA2N+2Q41ypGZDTvZpzODQsOHQ4cw= github.com/pierrec/lz4/v4 v4.1.22 h1:cKFw6uJDK+/gfw5BcDL0JL5aBsAFdsIT18eRtLj7VIU= github.com/pierrec/lz4/v4 v4.1.22/go.mod h1:gZWDp/Ze/IJXGXf23ltt2EXimqmTUXEy0GFuRQyBid4= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= From 9a11476894357a19ea1c202b51affbe5d5a08b90 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Fri, 15 Aug 2025 14:10:20 +0900 Subject: [PATCH 2/4] Cleanup --- enrichments/internal/elastic/span.go | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/enrichments/internal/elastic/span.go b/enrichments/internal/elastic/span.go index b4da46b..4407c2d 100644 --- a/enrichments/internal/elastic/span.go +++ b/enrichments/internal/elastic/span.go @@ -28,7 +28,6 @@ import ( "net/http" "net/url" "strconv" - "strings" "github.com/elastic/opentelemetry-lib/elasticattr" "github.com/elastic/opentelemetry-lib/enrichments/config" @@ -730,29 +729,6 @@ func isElasticTransaction(span ptrace.Span) bool { return false } -// parses string format `=val` -func getValueForKeyInString(str string, key string, separator rune, assignChar rune) string { - for { - str = strings.TrimSpace(str) - if str == "" { - break - } - kv := str - if sepIdx := strings.IndexRune(str, separator); sepIdx != -1 { - kv = strings.TrimSpace(str[:sepIdx]) - str = str[sepIdx+1:] - } else { - str = "" - } - equal := strings.IndexRune(kv, assignChar) - if equal != -1 && kv[:equal] == key { - return kv[equal+1:] - } - } - - return "" -} - func getHostPort( urlFull *url.URL, urlDomain string, urlPort int64, fallbackServerAddress string, fallbackServerPort int64, From 319a2d9f9771758fa5eb2535bf7622e7e436220f Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Fri, 15 Aug 2025 14:12:57 +0900 Subject: [PATCH 3/4] Less confusing --- enrichments/internal/elastic/span.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/enrichments/internal/elastic/span.go b/enrichments/internal/elastic/span.go index 4407c2d..061999c 100644 --- a/enrichments/internal/elastic/span.go +++ b/enrichments/internal/elastic/span.go @@ -43,7 +43,7 @@ import ( // defaultRepresentativeCount is the representative count to use for adjusting // sampled spans when a value could not be found from tracestate. Our default is -// to assume no sampling. +// to assume sampling all spans. const defaultRepresentativeCount = 1.0 // EnrichSpan adds Elastic specific attributes to the OTel span. From 0c60a74884459f02679ab368372c0d5f350d61ac Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Fri, 15 Aug 2025 14:23:23 +0900 Subject: [PATCH 4/4] Typo --- enrichments/internal/elastic/span.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/enrichments/internal/elastic/span.go b/enrichments/internal/elastic/span.go index 061999c..2e7d311 100644 --- a/enrichments/internal/elastic/span.go +++ b/enrichments/internal/elastic/span.go @@ -663,7 +663,7 @@ func (s *spanEventEnrichmentContext) enrich( // with a difference that representative count can also include // dynamically calculated representivity for non-probabilistic sampling. // In addition, the representative count defaults to 1 if the adjusted -// count is UNKNOWN or the th-value is invalid. +// count is UNKNOWN or the t-value is invalid. // // Def: https://opentelemetry.io/docs/specs/otel/trace/tracestate-probability-sampling/#converting-threshold-to-an-adjusted-count-sampling-rate //