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

maint: rename sent_reason_cache to kept_reason_cache #1346

Merged
merged 1 commit into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
16 changes: 8 additions & 8 deletions collect/cache/cuckooSentCache.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ type KeptTrace interface {
SpanEventCount() uint32
SpanLinkCount() uint32
SpanCount() uint32
SetSentReason(uint)
SentReason() uint
SetKeptReason(uint)
KeptReason() uint
}

func NewKeptTraceCacheEntry(t KeptTrace) *keptTraceCacheEntry {
Expand All @@ -53,7 +53,7 @@ func NewKeptTraceCacheEntry(t KeptTrace) *keptTraceCacheEntry {
spanEventCount: t.SpanEventCount(),
spanLinkCount: t.SpanLinkCount(),
spanCount: t.SpanCount(),
reason: uint32(t.SentReason()),
reason: uint32(t.KeptReason()),
}
}

Expand Down Expand Up @@ -155,7 +155,7 @@ type cuckooSentCache struct {

// This mutex is for managing kept traces
keptMut sync.Mutex
sentReasons *SentReasonsCache
keptReasons *KeptReasonsCache
}

// Make sure it implements TraceSentCache
Expand Down Expand Up @@ -188,7 +188,7 @@ func NewCuckooSentCache(cfg config.SampleCacheConfig, met metrics.Metrics) (Trac
dropped: dropped,
recentDroppedIDs: recentDroppedIDs,
cfg: cfg,
sentReasons: NewSentReasonsCache(met),
keptReasons: NewKeptReasonsCache(met),
done: make(chan struct{}),
}
go cache.monitor()
Expand Down Expand Up @@ -220,7 +220,7 @@ func (c *cuckooSentCache) Stop() {
func (c *cuckooSentCache) Record(trace KeptTrace, keep bool, reason string) {
if keep {
// record this decision in the sent record LRU for future spans
trace.SetSentReason(c.sentReasons.Set(reason))
trace.SetKeptReason(c.keptReasons.Set(reason))
sentRecord := NewKeptTraceCacheEntry(trace)

c.keptMut.Lock()
Expand Down Expand Up @@ -253,7 +253,7 @@ func (c *cuckooSentCache) CheckSpan(span *types.Span) (TraceSentRecord, string,
if sentRecord, found := c.kept.Get(span.TraceID); found {
// if we kept it, then this span being checked needs counting too
sentRecord.Count(span)
reason, _ := c.sentReasons.Get(uint(sentRecord.reason))
reason, _ := c.keptReasons.Get(uint(sentRecord.reason))
return sentRecord, reason, true
}
// we have no memory of this place
Expand Down Expand Up @@ -306,7 +306,7 @@ func (c *cuckooSentCache) CheckTrace(traceID string) (TraceSentRecord, string, b
c.keptMut.Lock()
defer c.keptMut.Unlock()
if sentRecord, found := c.kept.Get(traceID); found {
reason, _ := c.sentReasons.Get(uint(sentRecord.reason))
reason, _ := c.keptReasons.Get(uint(sentRecord.reason))
return sentRecord, reason, true
}
// we have no memory of this place
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ import (
"github.com/stretchr/testify/assert"
)

func TestSentReasonCache(t *testing.T) {
func TestKeptReasonCache(t *testing.T) {
s := &metrics.MockMetrics{}
s.Start()
c := cache.NewSentReasonsCache(s)
c := cache.NewKeptReasonsCache(s)
keys := make([]uint, 0)
entries := []string{"foo", "bar", "baz"}
for _, item := range entries {
Expand All @@ -29,7 +29,7 @@ func TestSentReasonCache(t *testing.T) {
}
}

func BenchmarkSentReasonCache_Set(b *testing.B) {
func BenchmarkKeptReasonCache_Set(b *testing.B) {
s := &metrics.MockMetrics{}
s.Start()
for _, numItems := range []int{10, 100, 1000, 10000, 100000} {
Expand All @@ -38,18 +38,18 @@ func BenchmarkSentReasonCache_Set(b *testing.B) {
entries[i] = randomString(50)
}
b.Run(strconv.Itoa(numItems), func(b *testing.B) {
cache := cache.NewSentReasonsCache(s)
cache := cache.NewKeptReasonsCache(s)
for i := 0; i < b.N; i++ {
cache.Set(entries[seededRand.Intn(numItems)])
}
})
}
}
func BenchmarkSentReasonCache_Get(b *testing.B) {
func BenchmarkKeptReasonCache_Get(b *testing.B) {
s := &metrics.MockMetrics{}
s.Start()
for _, numItems := range []int{10, 100, 1000, 10000, 100000} {
cache := cache.NewSentReasonsCache(s)
cache := cache.NewKeptReasonsCache(s)
for i := 0; i < numItems; i++ {
cache.Set(randomString(50))
}
Expand All @@ -61,13 +61,13 @@ func BenchmarkSentReasonCache_Get(b *testing.B) {
}
}

func BenchmarkSentReasonsCache_Get_Parallel(b *testing.B) {
func BenchmarkKeptReasonsCache_Get_Parallel(b *testing.B) {
for _, numGoroutines := range []int{1, 50, 300} {
for _, numUniqueEntries := range []int{50, 500, 2000} {
b.Run(fmt.Sprintf("entries%d-g%d", numUniqueEntries, numGoroutines), func(b *testing.B) {
s := &metrics.MockMetrics{}
s.Start()
cache := cache.NewSentReasonsCache(s)
cache := cache.NewKeptReasonsCache(s)

entries := make([]string, numUniqueEntries)
for i := 0; i < numUniqueEntries; i++ {
Expand Down Expand Up @@ -96,7 +96,7 @@ func BenchmarkSentReasonsCache_Get_Parallel(b *testing.B) {
}
}

func BenchmarkSentReasonsCache_Set_Parallel(b *testing.B) {
func BenchmarkKeptReasonsCache_Set_Parallel(b *testing.B) {
for _, numGoroutines := range []int{1, 50, 300} {
for _, numUniqueEntries := range []int{50, 500, 2000} {
b.Run(fmt.Sprintf("entries%d-g%d", numUniqueEntries, numGoroutines), func(b *testing.B) {
Expand All @@ -106,7 +106,7 @@ func BenchmarkSentReasonsCache_Set_Parallel(b *testing.B) {
for i := 0; i < numUniqueEntries; i++ {
entries[i] = randomString(50)
}
cache := cache.NewSentReasonsCache(s)
cache := cache.NewKeptReasonsCache(s)
wg := sync.WaitGroup{}
count := b.N / numGoroutines
if count == 0 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
// This is used to reduce the memory footprint of the trace cache.
// It is not concurrency-safe.

type SentReasonsCache struct {
type KeptReasonsCache struct {
Metrics metrics.Metrics

data []string
Expand All @@ -22,11 +22,11 @@ type SentReasonsCache struct {
hashSeed uint64
}

// NewSentReasonsCache returns a new SentReasonsCache.
func NewSentReasonsCache(metrics metrics.Metrics) *SentReasonsCache {
// NewKeptReasonsCache returns a new SentReasonsCache.
func NewKeptReasonsCache(metrics metrics.Metrics) *KeptReasonsCache {
metrics.Register("collect_sent_reasons_cache_entries", "histogram")

return &SentReasonsCache{
return &KeptReasonsCache{
Metrics: metrics,
keys: make(map[uint64]uint32),
hashSeed: rand.Uint64(),
Expand All @@ -35,7 +35,7 @@ func NewSentReasonsCache(metrics metrics.Metrics) *SentReasonsCache {

// Set adds a new reason to the cache, returning the key.
// The key is generated by incrementing a counter.
func (c *SentReasonsCache) Set(key string) uint {
func (c *KeptReasonsCache) Set(key string) uint {
// generate a hash
hash := wyhash.Hash([]byte(key), c.hashSeed)

Expand All @@ -50,7 +50,7 @@ func (c *SentReasonsCache) Set(key string) uint {
}

// Get returns a reason from the cache, if it exists.
func (c *SentReasonsCache) Get(key uint) (string, bool) {
func (c *KeptReasonsCache) Get(key uint) (string, bool) {
if key == 0 {
return "", false
}
Expand Down
12 changes: 6 additions & 6 deletions collect/collect.go
Original file line number Diff line number Diff line change
Expand Up @@ -480,12 +480,12 @@ func (i *InMemCollector) processSpan(sp *types.Span) {
trace := i.cache.Get(sp.TraceID)
if trace == nil {
// if the trace has already been sent, just pass along the span
if sr, sentReason, found := i.sampleTraceCache.CheckSpan(sp); found {
if sr, keptReason, found := i.sampleTraceCache.CheckSpan(sp); found {
i.Metrics.Increment("trace_sent_cache_hit")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe change the name of this metric? And are there other ones?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one is ok. This metric is for the TraceSentCache, not the keptReasonCache. Hopefully with this rename, things will be a bit more clear

// bump the count of records on this trace -- if the root span isn't
// the last late span, then it won't be perfect, but it will be better than
// having none at all
i.dealWithSentTrace(ctx, sr, sentReason, sp)
i.dealWithSentTrace(ctx, sr, keptReason, sp)
return
}
// trace hasn't already been sent (or this span is really old); let's
Expand Down Expand Up @@ -620,18 +620,18 @@ func (i *InMemCollector) ProcessSpanImmediately(sp *types.Span) (processed bool,
// 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(ctx context.Context, tr cache.TraceSentRecord, sentReason string, sp *types.Span) {
func (i *InMemCollector) dealWithSentTrace(ctx context.Context, tr cache.TraceSentRecord, keptReason string, sp *types.Span) {
_, span := otelutil.StartSpanMulti(ctx, i.Tracer, "dealWithSentTrace", map[string]interface{}{
"trace_id": sp.TraceID,
"sent_reason": sentReason,
"kept_reason": keptReason,
"hostname": i.hostname,
})
defer span.End()

if i.Config.GetAddRuleReasonToTrace() {
var metaReason string
if len(sentReason) > 0 {
metaReason = fmt.Sprintf("%s - late arriving span", sentReason)
if len(keptReason) > 0 {
metaReason = fmt.Sprintf("%s - late arriving span", keptReason)
} else {
metaReason = "late arriving span"
}
VinozzZ marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
10 changes: 5 additions & 5 deletions types/event.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ type Trace struct {
KeepSample bool
// Sent should only be changed if the changer holds the SendSampleLock
Sent bool
sentReason uint
keptReason uint

SendBy time.Time

Expand Down Expand Up @@ -114,12 +114,12 @@ func (t *Trace) SetSampleRate(rate uint) {
t.sampleRate = rate
}

func (t *Trace) SentReason() uint {
return t.sentReason
func (t *Trace) KeptReason() uint {
return t.keptReason
}

func (t *Trace) SetSentReason(reason uint) {
t.sentReason = reason
func (t *Trace) SetKeptReason(reason uint) {
t.keptReason = reason
}

// DescendantCount gets the number of descendants of all kinds currently in this trace
Expand Down
Loading