fix: do not use trace object during processTraceDecisions

collect/collect.go

@@ -36,7 +36,7 @@ const (
 	dropTraceDecisionTopic            = "trace_decision_dropped"
 	decisionMessageBufferSize         = 10_000
 	defaultDropDecisionTickerInterval = 1 * time.Second
-	defaultKeptDecisionTickerInterval = 100 * time.Millisecond
+	defaultKeptDecisionTickerInterval = 1 * time.Second
 )
 
 var ErrWouldBlock = errors.New("Dropping span as channel buffer is full. Span will not be processed and will be lost.")
@@ -869,9 +869,9 @@ func (i *InMemCollector) dealWithSentTrace(ctx context.Context, tr cache.TraceSe
 		td := TraceDecision{
 			TraceID:    sp.TraceID,
 			Kept:       tr.Kept(),
-			KeptReason: keptReason,
+			Reason:     keptReason,
 			SendReason: TraceSendLateSpan,
-			SampleRate: tr.Rate(),
+			Rate:       tr.Rate(),
 			Count:      uint32(tr.SpanCount()),
 			EventCount: uint32(tr.SpanEventCount()),
 			LinkCount:  uint32(tr.SpanLinkCount()),
@@ -1016,7 +1016,7 @@ func (i *InMemCollector) send(ctx context.Context, trace *types.Trace, td *Trace
 
 	i.Metrics.Increment("trace_send_kept")
 	// This will observe sample rate decisions only if the trace is kept
-	i.Metrics.Histogram("trace_kept_sample_rate", float64(td.SampleRate))
+	i.Metrics.Histogram("trace_kept_sample_rate", float64(td.Rate))
 
 	// ok, we're not dropping this trace; send all the spans
 	if i.Config.GetIsDryRun() && !td.Kept {
@@ -1027,7 +1027,7 @@ func (i *InMemCollector) send(ctx context.Context, trace *types.Trace, td *Trace
 	i.Logger.Info().WithFields(logFields).Logf("Sending trace")
 	i.outgoingTraces <- sendableTrace{
 		Trace:      trace,
-		reason:     td.KeptReason,
+		reason:     td.Reason,
 		sendReason: td.SendReason,
 		sampleKey:  td.SamplerKey,
 		shouldSend: td.Kept,
@@ -1415,12 +1415,14 @@ func (i *InMemCollector) processTraceDecisions(msg string, decisionType decision
 		}
 		toDelete.Add(decision.TraceID)
 
-		if decisionType == keptDecision {
-			trace.SetSampleRate(decision.SampleRate)
-			trace.KeepSample = decision.Kept
-		}
+		if _, _, ok := i.sampleTraceCache.CheckTrace(decision.TraceID); !ok {
+			if decisionType == keptDecision {
+				trace.SetSampleRate(decision.Rate)
+				trace.KeepSample = decision.Kept
+			}
 
-		i.sampleTraceCache.Record(trace, decision.Kept, decision.KeptReason)
+			i.sampleTraceCache.Record(&decision, decision.Kept, decision.Reason)
+		}
 
 		i.send(context.Background(), trace, &decision)
 	}
@@ -1487,10 +1489,10 @@ func (i *InMemCollector) makeDecision(ctx context.Context, trace *types.Trace, s
 	td := TraceDecision{
 		TraceID:         trace.ID(),
 		Kept:            shouldSend,
-		KeptReason:      reason,
+		Reason:          reason,
 		SamplerKey:      key,
 		SamplerSelector: samplerSelector,
-		SampleRate:      rate,
+		Rate:            rate,
 		SendReason:      sendReason,
 		Count:           trace.SpanCount(),
 		EventCount:      trace.SpanEventCount(),

collect/trace_decision.go

@@ -4,6 +4,8 @@ import (
 	"encoding/json"
 	"fmt"
 	"strings"
+
+	"github.com/honeycombio/refinery/collect/cache"
 )
 
 type decisionType int
@@ -81,23 +83,55 @@ func newKeptTraceDecision(msg string) ([]TraceDecision, error) {
 	return keptDecisions, nil
 }
 
+var _ cache.KeptTrace = &TraceDecision{}
+
 type TraceDecision struct {
 	TraceID string
 	// if we don'g need to immediately eject traces from the trace cache,
 	// we could remove this field. The TraceDecision type could be renamed to
 	// keptDecision
 	Kept            bool
-	SampleRate      uint
+	Rate            uint
 	SamplerKey      string `json:",omitempty"`
 	SamplerSelector string `json:",omitempty"`
 	SendReason      string
 	HasRoot         bool
-	KeptReason      string
+	Reason          string
 	Count           uint32 `json:",omitempty"` // number of spans in the trace
 	EventCount      uint32 `json:",omitempty"` // number of span events in the trace
 	LinkCount       uint32 `json:",omitempty"` // number of span links in the trace
+
+	keptReasonIdx uint `json:",omitempty"`
 }
 
 func (td *TraceDecision) DescendantCount() uint32 {
 	return td.Count + td.EventCount + td.LinkCount
 }
+
+func (td *TraceDecision) SpanCount() uint32 {
+	return td.Count
+}
+
+func (td *TraceDecision) SpanEventCount() uint32 {
+	return td.EventCount
+}
+
+func (td *TraceDecision) SpanLinkCount() uint32 {
+	return td.LinkCount
+}
+
+func (td *TraceDecision) SampleRate() uint {
+	return td.Rate
+}
+
+func (td *TraceDecision) ID() string {
+	return td.TraceID
+}
+
+func (td *TraceDecision) KeptReason() uint {
+	return td.keptReasonIdx
+}
+
+func (td *TraceDecision) SetKeptReason(reasonIdx uint) {
+	td.keptReasonIdx = reasonIdx
+}

collect/trace_decision_test.go

@@ -43,9 +43,9 @@ func TestNewKeptTraceDecision(t *testing.T) {
 	}{
 		{
 			name: "kept decision",
-			msg:  `[{"TraceID":"1", "Kept": true, "SampleRate": 100, "SendReason":"` + TraceSendGotRoot + `"}]`,
+			msg:  `[{"TraceID":"1", "Kept": true, "Rate": 100, "SendReason":"` + TraceSendGotRoot + `"}]`,
 			want: []TraceDecision{
-				{TraceID: "1", Kept: true, SampleRate: 100, SendReason: TraceSendGotRoot}},
+				{TraceID: "1", Kept: true, Rate: 100, SendReason: TraceSendGotRoot}},
 			wantErr: false,
 		},
 		{
@@ -122,12 +122,12 @@ func TestNewKeptDecisionMessage(t *testing.T) {
 				{
 					TraceID:    "1",
 					Kept:       true,
-					SampleRate: 100,
+					Rate:       100,
 					SendReason: TraceSendGotRoot,
-					KeptReason: "deterministic",
+					Reason:     "deterministic",
 				},
 			},
-			want:    `[{"TraceID":"1","Kept":true,"SampleRate":100,"SendReason":"trace_send_got_root","HasRoot":false,"KeptReason":"deterministic"}]`,
+			want:    `[{"TraceID":"1","Kept":true,"Rate":100,"SendReason":"trace_send_got_root","HasRoot":false,"Reason":"deterministic"}]`,
 			wantErr: false,
 		},
 		{