From 930e99485759c4b98f337b1417d6673a451bd436 Mon Sep 17 00:00:00 2001 From: Yingrong Zhao <22300958+VinozzZ@users.noreply.github.com> Date: Fri, 15 Nov 2024 15:41:01 -0500 Subject: [PATCH] fix: send all traffic through deterministic sampler during stress relief activated --- collect/collect.go | 5 ---- collect/stressRelief.go | 27 --------------------- collect/stress_relief_test.go | 44 ----------------------------------- 3 files changed, 76 deletions(-) diff --git a/collect/collect.go b/collect/collect.go index c0efe52f8a..bf6eac6841 100644 --- a/collect/collect.go +++ b/collect/collect.go @@ -802,11 +802,6 @@ func (i *InMemCollector) ProcessSpanImmediately(sp *types.Span) (processed bool, _, span := otelutil.StartSpanWith(context.Background(), i.Tracer, "collector.ProcessSpanImmediately", "trace_id", sp.TraceID) defer span.End() - if !i.StressRelief.ShouldSampleDeterministically(sp.TraceID) { - otelutil.AddSpanField(span, "nondeterministic", 1) - return false, false - } - var rate uint record, reason, found := i.sampleTraceCache.CheckSpan(sp) if !found { diff --git a/collect/stressRelief.go b/collect/stressRelief.go index 76bdea9df3..1b048f37bd 100644 --- a/collect/stressRelief.go +++ b/collect/stressRelief.go @@ -27,7 +27,6 @@ type StressReliever interface { Recalc() uint Stressed() bool GetSampleRate(traceID string) (rate uint, keep bool, reason string) - ShouldSampleDeterministically(traceID string) bool startstop.Starter } @@ -531,29 +530,3 @@ func (s *StressRelief) GetSampleRate(traceID string) (rate uint, keep bool, reas hash := wyhash.Hash([]byte(traceID), hashSeed) return uint(s.sampleRate), hash <= s.upperBound, "stress_relief/deterministic/" + s.reason } - -// ShouldSampleDeterministically returns true if the trace should be deterministically sampled. -// It uses the traceID to calculate a hash and then divides it by the maximum possible value -// to get a percentage. If the percentage is less than the deterministic fraction, it returns true. -func (s *StressRelief) ShouldSampleDeterministically(traceID string) bool { - samplePercentage := s.deterministicFraction() - hash := wyhash.Hash([]byte(traceID), hashSeed) - - return float64(hash)/float64(math.MaxUint64)*100 < float64(samplePercentage) -} - -// deterministicFraction returns the fraction of traces that should be deterministic sampled -// It calculates the result by using the stress level as the fraction between the activation -// level and 100%. The result is rounded to the nearest integer. -// -// for example: -// - if the stress level is 90 and the activation level is 80, the result will be 50 -// - meaning that 50% of the traces should be deterministic sampled -func (s *StressRelief) deterministicFraction() uint { - if s.overallStressLevel < s.activateLevel { - return 0 - } - - // round to the nearest integer - return uint(float64(s.overallStressLevel-s.activateLevel)/float64(100-s.activateLevel)*100 + 0.5) -} diff --git a/collect/stress_relief_test.go b/collect/stress_relief_test.go index 2e88dd5bc8..53660283a8 100644 --- a/collect/stress_relief_test.go +++ b/collect/stress_relief_test.go @@ -3,8 +3,6 @@ package collect import ( "context" "fmt" - "math" - "math/rand" "testing" "time" @@ -304,48 +302,6 @@ func TestStressRelief_OverallStressLevel_EnableTraceLocality(t *testing.T) { assert.False(t, sr.stressed) } -// TestStressRelief_Sample tests that traces are sampled deterministically -// by traceID. -// The test generates 10000 traceIDs and checks that the sampling rate is -// within 10% of the expected value. -func TestStressRelief_ShouldSampleDeterministically(t *testing.T) { - traceCount := 10000 - traceIDs := make([]string, 0, traceCount) - for i := 0; i < traceCount; i++ { - traceIDs = append(traceIDs, fmt.Sprintf("%016x%016x", rand.Int63(), rand.Int63())) - } - - sr := &StressRelief{ - overallStressLevel: 90, - activateLevel: 60, - } - - var sampled int - var dropped int - var sampledTraceID string - var droppedTraceID string - for _, traceID := range traceIDs { - if sr.ShouldSampleDeterministically(traceID) { - sampled++ - if sampledTraceID == "" { - sampledTraceID = traceID - } - } else { - if droppedTraceID == "" { - droppedTraceID = traceID - } - dropped++ - } - } - - difference := float64(sampled)/float64(traceCount)*100 - float64(sr.deterministicFraction()) - require.LessOrEqual(t, math.Floor(math.Abs(float64(difference))), float64(10), sampled) - - // make sure that the same traceID always gets the same result - require.True(t, sr.ShouldSampleDeterministically(sampledTraceID)) - require.False(t, sr.ShouldSampleDeterministically(droppedTraceID)) -} - func newStressRelief(t *testing.T, clock clockwork.Clock, channel pubsub.PubSub) (*StressRelief, func()) { // Create a new StressRelief object metric := &metrics.MockMetrics{}