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

[chore][extensions/jaegerremotesampling]: replace github.com/tilinna/clock with github.com/jonboulle/clockwork #34224

Merged
merged 2 commits into from
Jul 23, 2024
Merged

[chore][extensions/jaegerremotesampling]: replace github.com/tilinna/clock with github.com/jonboulle/clockwork #34224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7b8e9bb
[chore][extensions/jaegerremotesampling]: replace github.com/tilinna/…
odubajDT Jul 23, 2024
36f4e29
go mod tidy
odubajDT Jul 23, 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
1 change: 1 addition & 0 deletions cmd/otelcontribcol/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,7 @@ require (
github.com/jcmturner/rpc/v2 v2.0.3 // indirect
github.com/jmespath/go-jmespath v0.4.0 // indirect
github.com/joeshaw/multierror v0.0.0-20140124173710-69b34d4ec901 // indirect
github.com/jonboulle/clockwork v0.4.0 // indirect
github.com/josharian/intern v1.0.0 // indirect
github.com/jpillora/backoff v1.0.0 // indirect
github.com/json-iterator/go v1.1.12 // indirect
Expand Down
2 changes: 1 addition & 1 deletion extension/jaegerremotesampling/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ go 1.21.0
require (
github.com/fortytw2/leaktest v1.3.0
github.com/jaegertracing/jaeger v1.59.0
github.com/jonboulle/clockwork v0.4.0
github.com/open-telemetry/opentelemetry-collector-contrib/internal/common v0.105.0
github.com/stretchr/testify v1.9.0
github.com/tilinna/clock v1.1.0
go.opentelemetry.io/collector/component v0.105.1-0.20240717163034-43ed6184f9fe
go.opentelemetry.io/collector/config/configgrpc v0.105.1-0.20240717163034-43ed6184f9fe
go.opentelemetry.io/collector/config/confighttp v0.105.1-0.20240717163034-43ed6184f9fe
Expand Down
4 changes: 2 additions & 2 deletions extension/jaegerremotesampling/go.sum
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions extension/jaegerremotesampling/internal/remote_strategy_cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"time"

"github.com/jaegertracing/jaeger/proto-gen/api_v2"
"github.com/tilinna/clock"
"github.com/jonboulle/clockwork"
)

type serviceStrategyCache interface {
Expand Down Expand Up @@ -78,7 +78,7 @@ func (c *serviceStrategyTTLCache) put(
defer c.rw.Unlock()
c.items[serviceName] = serviceStrategyCacheEntry{
strategyResponse: response,
retrievedAt: clock.Now(ctx),
retrievedAt: clockwork.FromContext(ctx).Now(),
}
}

Expand All @@ -89,10 +89,10 @@ func (c *serviceStrategyTTLCache) periodicallyClearCache(
ctx context.Context,
schedulingPeriod time.Duration,
) {
ticker := clock.NewTicker(ctx, schedulingPeriod)
ticker := clockwork.FromContext(ctx).NewTicker(schedulingPeriod)
for {
select {
case <-ticker.C:
case <-ticker.Chan():
c.rw.Lock()
newItems := make(map[string]serviceStrategyCacheEntry, initialRemoteResponseCacheSize)
for serviceName, item := range c.items {
Expand All @@ -115,7 +115,7 @@ func (c *serviceStrategyTTLCache) Close() error {
}

func (c *serviceStrategyTTLCache) staleItem(ctx context.Context, item serviceStrategyCacheEntry) bool {
return clock.Now(ctx).After(item.retrievedAt.Add(c.itemTTL))
return clockwork.FromContext(ctx).Now().After(item.retrievedAt.Add(c.itemTTL))
}

type noopStrategyCache struct{}
Expand Down
37 changes: 21 additions & 16 deletions extension/jaegerremotesampling/internal/remote_strategy_cache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import (

"github.com/fortytw2/leaktest"
"github.com/jaegertracing/jaeger/proto-gen/api_v2"
"github.com/jonboulle/clockwork"
"github.com/stretchr/testify/assert"
"github.com/tilinna/clock"
)

const cacheTestItemTTL = 50 * time.Millisecond
Expand Down Expand Up @@ -48,9 +48,11 @@ var testStrategyResponseB = &api_v2.SamplingStrategyResponse{

func Test_serviceStrategyCache_ReadWriteSequence(t *testing.T) {
testTime := time.Date(2023, 1, 1, 10, 0, 0, 0, time.UTC)
mock := clock.NewMock(testTime)
ctx, cfn := mock.DeadlineContext(context.Background(), testTime.Add(3*time.Minute))
defer cfn()
mock := clockwork.NewFakeClockAt(testTime)
_ = mock.After(3 * time.Minute)

ctx, cancel := context.WithCancel(clockwork.AddToContext(context.Background(), mock))
defer cancel()

cache := newServiceStrategyCache(cacheTestItemTTL).(*serviceStrategyTTLCache)
defer func() {
Expand Down Expand Up @@ -91,7 +93,7 @@ func Test_serviceStrategyCache_ReadWriteSequence(t *testing.T) {
}, cache.items["fooSvc"])

// advance time (still within TTL time range)
mock.Add(20 * time.Millisecond)
mock.Advance(20 * time.Millisecond)

// the written item is still available
result, ok = cache.get(ctx, "fooSvc")
Expand All @@ -102,7 +104,7 @@ func Test_serviceStrategyCache_ReadWriteSequence(t *testing.T) {
assert.Nil(t, result)

// advance time (just before end of TTL time range)
mock.Add(30 * time.Millisecond)
mock.Advance(30 * time.Millisecond)

// the written item is still available
result, ok = cache.get(ctx, "fooSvc")
Expand All @@ -113,7 +115,7 @@ func Test_serviceStrategyCache_ReadWriteSequence(t *testing.T) {
assert.Nil(t, result)

// advance time (across TTL range)
mock.Add(1 * time.Millisecond)
mock.Advance(1 * time.Millisecond)

// the (now stale) cached item is no longer available
result, ok = cache.get(ctx, "fooSvc")
Expand All @@ -131,9 +133,11 @@ func Test_serviceStrategyCache_ReadWriteSequence(t *testing.T) {

func Test_serviceStrategyCache_WritesUpdateTimestamp(t *testing.T) {
startTime := time.Date(2023, 1, 1, 10, 0, 0, 0, time.UTC)
mock := clock.NewMock(startTime)
ctx, cfn := mock.DeadlineContext(context.Background(), startTime.Add(3*time.Minute))
defer cfn()
mock := clockwork.NewFakeClockAt(startTime)
_ = mock.After(3 * time.Minute)

ctx, cancel := context.WithCancel(clockwork.AddToContext(context.Background(), mock))
defer cancel()

cache := newServiceStrategyCache(cacheTestItemTTL).(*serviceStrategyTTLCache)
defer func() {
Expand All @@ -149,12 +153,12 @@ func Test_serviceStrategyCache_WritesUpdateTimestamp(t *testing.T) {
assert.Nil(t, result)

// perform a write for barSvc at startTime + 10ms
firstWriteTime := mock.Add(10 * time.Millisecond)
mock.Advance(10 * time.Millisecond)
cache.put(ctx, "barSvc", testStrategyResponseA)

// whitebox assert for internal timestamp tracking
assert.Equal(t, serviceStrategyCacheEntry{
retrievedAt: firstWriteTime,
retrievedAt: mock.Now(),
strategyResponse: testStrategyResponseA,
}, cache.items["barSvc"])

Expand All @@ -167,7 +171,7 @@ func Test_serviceStrategyCache_WritesUpdateTimestamp(t *testing.T) {
assert.Equal(t, testStrategyResponseA, result)

// advance time (still within TTL time range)
mock.Add(10 * time.Millisecond)
mock.Advance(10 * time.Millisecond)

// the written item is still available
result, ok = cache.get(ctx, "fooSvc")
Expand All @@ -178,8 +182,9 @@ func Test_serviceStrategyCache_WritesUpdateTimestamp(t *testing.T) {
assert.Equal(t, testStrategyResponseA, result)

// perform a write for barSvc at startTime + 30ms (still within TTL, but we retain this more recent data)
secondWriteTime := mock.Add(10 * time.Millisecond)
mock.Advance(10 * time.Millisecond)
cache.put(ctx, "barSvc", testStrategyResponseB)
secondWriteTime := mock.Now()

// whitebox assert for internal timestamp tracking (post-write, still-fresh cache entry replaced with newer data)
assert.Equal(t, serviceStrategyCacheEntry{
Expand All @@ -196,7 +201,7 @@ func Test_serviceStrategyCache_WritesUpdateTimestamp(t *testing.T) {
assert.Equal(t, testStrategyResponseB, result)

// advance time (to end of what is now a new/full TTL for the new fresh item)
mock.Add(cacheTestItemTTL)
mock.Advance(cacheTestItemTTL)

result, ok = cache.get(ctx, "fooSvc")
assert.False(t, ok)
Expand All @@ -206,7 +211,7 @@ func Test_serviceStrategyCache_WritesUpdateTimestamp(t *testing.T) {
assert.Equal(t, testStrategyResponseB, result)

// advance time beyond the newer item's TTL
mock.Add(1)
mock.Advance(1)

// the (now stale) cached item is no longer available
result, ok = cache.get(ctx, "fooSvc")
Expand Down