Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: do not send sample rate in dry run #611

Merged
merged 2 commits into from
Feb 24, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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