Skip to content

Commit

Permalink
fix: send all traffic through deterministic sampler during stress rel…
Browse files Browse the repository at this point in the history
…ief activated (#1433)

## Which problem is this PR solving?

- fix: #1391 

## Short description of the changes

- remove logic for sending fraction of all traffic through normal
operation during stress

Co-authored-by: Mike Goldsmith <goldsmith.mike@gmail.com>
  • Loading branch information
VinozzZ and MikeGoldsmith authored Nov 18, 2024
1 parent 6beef3d commit 14cc28a
Show file tree
Hide file tree
Showing 3 changed files with 0 additions and 76 deletions.
5 changes: 0 additions & 5 deletions collect/collect.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
27 changes: 0 additions & 27 deletions collect/stressRelief.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}
44 changes: 0 additions & 44 deletions collect/stress_relief_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ package collect
import (
"context"
"fmt"
"math"
"math/rand"
"testing"
"time"

Expand Down Expand Up @@ -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{}
Expand Down

0 comments on commit 14cc28a

Please sign in to comment.