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.

Sign up for GitHub

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: flaky TestOriginalSampleRateIsNotedInMetaField #991

Merged
merged 3 commits into from
Feb 14, 2024
Merged

fix: flaky TestOriginalSampleRateIsNotedInMetaField #991

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
995128b
fix: don't assume second element, look at end of slice
robbkidd Feb 2, 2024
87cdf08
require the event with no upstream sample rate
robbkidd Feb 13, 2024
a0ae7c3
rub a little bit more require on test setup
robbkidd Feb 14, 2024
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
53 changes: 34 additions & 19 deletions collect/collect_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/facebookgo/inject"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/honeycombio/refinery/collect/cache"
"github.com/honeycombio/refinery/config"
Expand Down Expand Up @@ -124,10 +125,15 @@ func TestAddRootSpan(t *testing.T) {
// happening upstream of refinery. Writing down what got sent to refinery
// will help people figure out what is going on.
func TestOriginalSampleRateIsNotedInMetaField(t *testing.T) {
// The sample rate applied by Refinery in this test's config.
const expectedDeterministicSampleRate = int(2)
// The sample rate happening upstream of Refinery.
const originalSampleRate = uint(50)

conf := &config.MockConfig{
GetSendDelayVal: 0,
GetTraceTimeoutVal: 60 * time.Second,
GetSamplerTypeVal: &config.DeterministicSamplerConfig{SampleRate: 2},
GetSamplerTypeVal: &config.DeterministicSamplerConfig{SampleRate: expectedDeterministicSampleRate},
SendTickerVal: 2 * time.Millisecond,
ParentIdFieldNames: []string{"trace.parent_id", "parentId"},
}
Expand All @@ -147,7 +153,7 @@ func TestOriginalSampleRateIsNotedInMetaField(t *testing.T) {
go coll.collect()
defer coll.Stop()

// Spin until a sample gets triggered
// Generate events until one is sampled and appears on the transmission queue for sending.
sendAttemptCount := 0
for getEventsLength(transmission) < 1 {
sendAttemptCount++
Expand All @@ -156,41 +162,50 @@ func TestOriginalSampleRateIsNotedInMetaField(t *testing.T) {
Event: types.Event{
Dataset: "aoeu",
APIKey: legacyAPIKey,
SampleRate: 50,
SampleRate: originalSampleRate,
Data: make(map[string]interface{}),
},
}
coll.AddSpan(span)
err := coll.AddSpan(span)
require.NoError(t, err, "must be able to add the span")
time.Sleep(conf.SendTickerVal * 5)
}

transmission.Mux.RLock()
assert.Greater(t, len(transmission.Events), 0, "should be at least one event transmitted")
assert.Equal(t, uint(50), transmission.Events[0].Data["meta.refinery.original_sample_rate"],
"metadata should be populated with original sample rate")
require.Greater(t, len(transmission.Events), 0,
"At least one event should have been sampled and transmitted by now for us to make assertions upon.")
upstreamSampledEvent := transmission.Events[0]
transmission.Mux.RUnlock()

span := &types.Span{
assert.Equal(t, originalSampleRate, upstreamSampledEvent.Data["meta.refinery.original_sample_rate"],
"metadata should be populated with original sample rate")
assert.Equal(t, originalSampleRate*uint(expectedDeterministicSampleRate), upstreamSampledEvent.SampleRate,
"sample rate for the event should be the original sample rate multiplied by the deterministic sample rate")

// Generate one more event with no upstream sampling applied.
err = coll.AddSpan(&types.Span{
TraceID: fmt.Sprintf("trace-%v", 1000),
Event: types.Event{
Dataset: "aoeu",
Dataset: "no-upstream-sampling",
APIKey: legacyAPIKey,
SampleRate: 0,
SampleRate: 0, // no upstream sampling
Data: make(map[string]interface{}),
},
}
})
require.NoError(t, err, "must be able to add the span")

coll.AddSpan(span)
assert.Eventually(t, func() bool {
// Find the Refinery-sampled-and-sent event that had no upstream sampling which
// should be the last event on the transmission queue.
var noUpstreamSampleRateEvent *types.Event
require.Eventually(t, func() bool {
transmission.Mux.RLock()
defer transmission.Mux.RUnlock()
return len(transmission.Events) > 1
}, 5*time.Second, conf.SendTickerVal*2, "should be at least two events transmitted")
noUpstreamSampleRateEvent = transmission.Events[len(transmission.Events)-1]
return noUpstreamSampleRateEvent.Dataset == "no-upstream-sampling"
}, 5*time.Second, conf.SendTickerVal*2, "the event with no upstream sampling should have appeared in the transmission queue by now")

transmission.Mux.RLock()
assert.Nil(t, transmission.Events[1].Data["meta.refinery.original_sample_rate"],
"metadata should not be populated when zero")
transmission.Mux.RUnlock()
assert.Nil(t, noUpstreamSampleRateEvent.Data["meta.refinery.original_sample_rate"],
"original sample rate should not be set in metadata when original sample rate is zero")
}

// HoneyComb treats a missing or 0 SampleRate the same as 1, but
Expand Down
Loading