Skip to content

Commit

Permalink
fix: do not send sample rate in dry run
Browse files Browse the repository at this point in the history
  • Loading branch information
fchikwekwe committed Feb 23, 2023
1 parent 60fc9c2 commit 18da695
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 12 deletions.
26 changes: 17 additions & 9 deletions collect/collect.go
Original file line number Diff line number Diff line change
Expand Up @@ -557,15 +557,16 @@ func (i *InMemCollector) ProcessSpanImmediately(sp *types.Span, keep bool, sampl
// ok, we're sending it, so decorate it first
sp.Event.Data["meta.stressed"] = true
sp.Event.Data["meta.refinery.reason"] = reason
mergeTraceAndSpanSampleRates(sp, sampleRate)
mergeTraceAndSpanSampleRates(sp, sampleRate, i.Config.GetIsDryRun())
i.Transmission.EnqueueSpan(sp)
}

// dealWithSentTrace handles a span that has arrived after the sampling decision
// on the trace has already been made, and it obeys that decision by either
// sending the span immediately or dropping it.
func (i *InMemCollector) dealWithSentTrace(keep bool, sampleRate uint, spanCount uint, sp *types.Span) {
if i.Config.GetIsDryRun() {
isDryRun := i.Config.GetIsDryRun()
if isDryRun {
field := i.Config.GetDryRunFieldName()
// if dry run mode is enabled, we keep all traces and mark the spans with the sampling decision
sp.Data[field] = keep
Expand All @@ -577,7 +578,7 @@ func (i *InMemCollector) dealWithSentTrace(keep bool, sampleRate uint, spanCount
}
if keep {
i.Logger.Debug().WithField("trace_id", sp.TraceID).Logf("Sending span because of previous decision to send trace")
mergeTraceAndSpanSampleRates(sp, sampleRate)
mergeTraceAndSpanSampleRates(sp, sampleRate, isDryRun)
// if this span is a late root span, possibly update it with our current span count
if i.Config.GetAddSpanCountToRoot() && isRootSpan(sp) {
sp.Data["meta.span_count"] = int64(spanCount)
Expand All @@ -588,26 +589,32 @@ func (i *InMemCollector) dealWithSentTrace(keep bool, sampleRate uint, spanCount
i.Logger.Debug().WithField("trace_id", sp.TraceID).Logf("Dropping span because of previous decision to drop trace")
}

func mergeTraceAndSpanSampleRates(sp *types.Span, traceSampleRate uint) {
func mergeTraceAndSpanSampleRates(sp *types.Span, traceSampleRate uint, dryRunMode bool) {
tempSampleRate := sp.SampleRate
if traceSampleRate != 1 {
// When the sample rate from the trace is not 1 that means we are
// going to mangle the span sample rate. Write down the original sample
// rate so that that information is more easily recovered
sp.Data["meta.refinery.original_sample_rate"] = sp.SampleRate
}

if sp.SampleRate < 1 {
if tempSampleRate < 1 {
// See https://docs.honeycomb.io/manage-data-volume/sampling/
// SampleRate is the denominator of the ratio of sampled spans
// HoneyComb treats a missing or 0 SampleRate the same as 1, but
// behaves better/more consistently if the SampleRate is explicitly
// set instead of inferred
sp.SampleRate = 1
tempSampleRate = 1
}

// if spans are already sampled, take that in to account when computing
// the final rate
sp.SampleRate *= traceSampleRate
if dryRunMode {
sp.Data["meta.dryrun.sample_rate"] = tempSampleRate * traceSampleRate
sp.SampleRate = tempSampleRate
} else {
sp.SampleRate = tempSampleRate * traceSampleRate
}
}

func isRootSpan(sp *types.Span) bool {
Expand Down Expand Up @@ -701,14 +708,15 @@ func (i *InMemCollector) send(trace *types.Trace, reason string) {
sp.Data["meta.span_count"] = int64(trace.DescendantCount())
}

if i.Config.GetIsDryRun() {
isDryRun := i.Config.GetIsDryRun()
if isDryRun {
field := i.Config.GetDryRunFieldName()
sp.Data[field] = shouldSend
}
if i.hostname != "" {
sp.Data["meta.refinery.local_hostname"] = i.hostname
}
mergeTraceAndSpanSampleRates(sp, trace.SampleRate)
mergeTraceAndSpanSampleRates(sp, trace.SampleRate, isDryRun)
i.Transmission.EnqueueSpan(sp)
}
}
Expand Down
14 changes: 11 additions & 3 deletions collect/collect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,14 +336,17 @@ func TestDryRunMode(t *testing.T) {
var traceID2 = "def456"
var traceID3 = "ghi789"
// sampling decisions based on trace ID
_, keepTraceID1, _ := sampler.GetSampleRate(&types.Trace{TraceID: traceID1})
sampleRate1, keepTraceID1, _ := sampler.GetSampleRate(&types.Trace{TraceID: traceID1})
// would be dropped if dry run mode was not enabled
assert.False(t, keepTraceID1)
_, keepTraceID2, _ := sampler.GetSampleRate(&types.Trace{TraceID: traceID2})
assert.Equal(t, uint(10), sampleRate1)
sampleRate2, keepTraceID2, _ := sampler.GetSampleRate(&types.Trace{TraceID: traceID2})
assert.True(t, keepTraceID2)
_, keepTraceID3, _ := sampler.GetSampleRate(&types.Trace{TraceID: traceID3})
assert.Equal(t, uint(10), sampleRate2)
sampleRate3, keepTraceID3, _ := sampler.GetSampleRate(&types.Trace{TraceID: traceID3})
// would be dropped if dry run mode was not enabled
assert.False(t, keepTraceID3)
assert.Equal(t, uint(10), sampleRate3)

span := &types.Span{
TraceID: traceID1,
Expand Down Expand Up @@ -394,6 +397,11 @@ func TestDryRunMode(t *testing.T) {
// both spans should be marked with the sampling decision
assert.Equal(t, keepTraceID2, transmission.Events[1].Data[field], "field should match sampling decision for its trace ID")
assert.Equal(t, keepTraceID2, transmission.Events[2].Data[field], "field should match sampling decision for its trace ID")
// check that meta value associated with dry run mode is properly applied
assert.Equal(t, uint(10), transmission.Events[1].Data["meta.dryrun.sample_rate"])
// check expected sampleRate against span data
assert.Equal(t, sampleRate1, transmission.Events[0].Data["meta.dryrun.sample_rate"])
assert.Equal(t, sampleRate2, transmission.Events[1].Data["meta.dryrun.sample_rate"])
transmission.Mux.RUnlock()

span = &types.Span{
Expand Down

0 comments on commit 18da695

Please sign in to comment.