From 31b0c77101cf731215ad23587013ba2bdba6e9e2 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Tue, 23 Feb 2021 03:33:22 +0000 Subject: [PATCH 1/7] Stagger timestamps in exact aggregator tests Fixes #1559. --- sdk/metric/aggregator/exact/exact_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/sdk/metric/aggregator/exact/exact_test.go b/sdk/metric/aggregator/exact/exact_test.go index daa158d6b5e..6dd2b9b4652 100644 --- a/sdk/metric/aggregator/exact/exact_test.go +++ b/sdk/metric/aggregator/exact/exact_test.go @@ -117,6 +117,12 @@ type mergeTest struct { absolute bool } +func advance() { + before := time.Now() + for time.Now() != before { + } +} + func (mt *mergeTest) run(t *testing.T, profile aggregatortest.Profile) { descriptor := aggregatortest.NewAggregatorTest(metric.ValueRecorderInstrumentKind, profile.NumberKind) agg1, agg2, ckpt1, ckpt2 := new4() @@ -130,15 +136,18 @@ func (mt *mergeTest) run(t *testing.T, profile aggregatortest.Profile) { x2 := profile.Random(+1) all.Append(x2) + advance() aggregatortest.CheckedUpdate(t, agg2, x2, descriptor) if !mt.absolute { y1 := profile.Random(-1) all.Append(y1) + advance() aggregatortest.CheckedUpdate(t, agg1, y1, descriptor) y2 := profile.Random(-1) all.Append(y2) + advance() aggregatortest.CheckedUpdate(t, agg2, y2, descriptor) } } From b26e666444dff286b2dfb525185763aecef9d141 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Tue, 23 Feb 2021 03:35:42 +0000 Subject: [PATCH 2/7] Missed one --- CHANGELOG.md | 1 + sdk/metric/aggregator/exact/exact_test.go | 1 + 2 files changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2df2c302c9..acd62620869 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Rename MaxEventsPerSpan, MaxAttributesPerSpan and MaxLinksPerSpan to EventCountLimit, AttributeCountLimit and LinkCountLimit, and move these fieds into SpanLimits. (#1535) - Renamed the `otel/label` package to `otel/attribute`. (#1541) - Vendor the Jaeger exporter's dependency on Apache Thrift. (#1551) +- Stagger timestamps in exact aggregator tests. (#1568) ### Added diff --git a/sdk/metric/aggregator/exact/exact_test.go b/sdk/metric/aggregator/exact/exact_test.go index 6dd2b9b4652..b4905cfc911 100644 --- a/sdk/metric/aggregator/exact/exact_test.go +++ b/sdk/metric/aggregator/exact/exact_test.go @@ -132,6 +132,7 @@ func (mt *mergeTest) run(t *testing.T, profile aggregatortest.Profile) { for i := 0; i < mt.count; i++ { x1 := profile.Random(+1) all.Append(x1) + advance() aggregatortest.CheckedUpdate(t, agg1, x1, descriptor) x2 := profile.Random(+1) From f5b636caaa0c1bc2254bc5820c0ebe4a5acddade Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Tue, 23 Feb 2021 03:42:12 +0000 Subject: [PATCH 3/7] Yield while you wait --- sdk/metric/aggregator/exact/exact_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/metric/aggregator/exact/exact_test.go b/sdk/metric/aggregator/exact/exact_test.go index b4905cfc911..d93bbe5419a 100644 --- a/sdk/metric/aggregator/exact/exact_test.go +++ b/sdk/metric/aggregator/exact/exact_test.go @@ -120,6 +120,7 @@ type mergeTest struct { func advance() { before := time.Now() for time.Now() != before { + time.Sleep(0) } } From 45fefbc46afc816b85bdc4d4d5285c8dcab8bf36 Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Tue, 23 Feb 2021 03:48:05 +0000 Subject: [PATCH 4/7] Just sleep for a teeny tiny bit --- sdk/metric/aggregator/exact/exact_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/sdk/metric/aggregator/exact/exact_test.go b/sdk/metric/aggregator/exact/exact_test.go index d93bbe5419a..10e0c6152b1 100644 --- a/sdk/metric/aggregator/exact/exact_test.go +++ b/sdk/metric/aggregator/exact/exact_test.go @@ -118,10 +118,7 @@ type mergeTest struct { } func advance() { - before := time.Now() - for time.Now() != before { - time.Sleep(0) - } + time.Sleep(time.Nanosecond) } func (mt *mergeTest) run(t *testing.T, profile aggregatortest.Profile) { From 7e631a1c3ca09f158a80dcd27a4192c49888a23a Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Tue, 23 Feb 2021 03:59:56 +0000 Subject: [PATCH 5/7] Oops, wrong PR in Changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index acd62620869..1d94d7fdedf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Rename MaxEventsPerSpan, MaxAttributesPerSpan and MaxLinksPerSpan to EventCountLimit, AttributeCountLimit and LinkCountLimit, and move these fieds into SpanLimits. (#1535) - Renamed the `otel/label` package to `otel/attribute`. (#1541) - Vendor the Jaeger exporter's dependency on Apache Thrift. (#1551) -- Stagger timestamps in exact aggregator tests. (#1568) +- Stagger timestamps in exact aggregator tests. (#1569) ### Added From 0eca321439bb63f10e205e6d9f14b6cebc28f3ac Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Tue, 23 Feb 2021 15:24:15 +0000 Subject: [PATCH 6/7] Make sure that *some* time passes --- sdk/metric/aggregator/exact/exact_test.go | 16 ++++++++++++---- sdk/metric/processor/basic/basic_test.go | 6 ++++-- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/sdk/metric/aggregator/exact/exact_test.go b/sdk/metric/aggregator/exact/exact_test.go index 10e0c6152b1..d02f3905583 100644 --- a/sdk/metric/aggregator/exact/exact_test.go +++ b/sdk/metric/aggregator/exact/exact_test.go @@ -70,10 +70,12 @@ func (ut *updateTest) run(t *testing.T, profile aggregatortest.Profile) { for i := 0; i < ut.count; i++ { x := profile.Random(+1) all.Append(x) + advance() aggregatortest.CheckedUpdate(t, agg, x, descriptor) y := profile.Random(-1) all.Append(y) + advance() aggregatortest.CheckedUpdate(t, agg, y, descriptor) } @@ -167,7 +169,7 @@ func (mt *mergeTest) run(t *testing.T, profile aggregatortest.Profile) { received.Append(s.Number) if i > 0 { - require.False(t, pts[i-1].Time.After(pts[i].Time)) + require.True(t, pts[i-1].Time.Before(pts[i].Time)) } } @@ -210,9 +212,11 @@ func TestExactErrors(t *testing.T) { descriptor := aggregatortest.NewAggregatorTest(metric.ValueRecorderInstrumentKind, profile.NumberKind) + advance() aggregatortest.CheckedUpdate(t, agg, number.Number(0), descriptor) if profile.NumberKind == number.Float64Kind { + advance() aggregatortest.CheckedUpdate(t, agg, number.NewFloat64Number(math.NaN()), descriptor) } require.NoError(t, agg.SynchronizedMove(ckpt, descriptor)) @@ -261,11 +265,13 @@ func TestExactFloat64(t *testing.T) { for _, f := range fpsf(1) { all.Append(number.NewFloat64Number(f)) + advance() aggregatortest.CheckedUpdate(t, agg, number.NewFloat64Number(f), descriptor) } for _, f := range fpsf(-1) { all.Append(number.NewFloat64Number(f)) + advance() aggregatortest.CheckedUpdate(t, agg, number.NewFloat64Number(f), descriptor) } @@ -290,11 +296,11 @@ func TestExactFloat64(t *testing.T) { for i := 0; i < len(po); i++ { require.Equal(t, all.Points()[i], po[i].Number, "Wrong point at position %d", i) if i > 0 { - require.False(t, po[i-1].Time.After(po[i].Time)) + require.True(t, po[i-1].Time.Before(po[i].Time)) } } - require.False(t, po[0].Time.Before(startTime)) - require.False(t, po[len(po)-1].Time.After(endTime)) + require.True(t, po[0].Time.After(startTime)) + require.True(t, po[len(po)-1].Time.Before(endTime)) } func TestSynchronizedMoveReset(t *testing.T) { @@ -319,12 +325,14 @@ func TestMergeBehavior(t *testing.T) { for i := 0; i < 100; i++ { x1 := profile.Random(+1) all.Append(x1) + advance() aggregatortest.CheckedUpdate(t, agg1, x1, descriptor) } for i := 0; i < 100; i++ { x2 := profile.Random(+1) all.Append(x2) + advance() aggregatortest.CheckedUpdate(t, agg2, x2, descriptor) } diff --git a/sdk/metric/processor/basic/basic_test.go b/sdk/metric/processor/basic/basic_test.go index d25827bf703..1e8fc4e6c5a 100644 --- a/sdk/metric/processor/basic/basic_test.go +++ b/sdk/metric/processor/basic/basic_test.go @@ -316,7 +316,9 @@ func TestBasicInconsistent(t *testing.T) { func TestBasicTimestamps(t *testing.T) { beforeNew := time.Now() + time.Sleep(time.Nanosecond) b := basic.New(processorTest.AggregatorSelector(), export.StatelessExportKindSelector()) + time.Sleep(time.Nanosecond) afterNew := time.Now() desc := metric.NewDescriptor("inst", metric.CounterInstrumentKind, number.Int64Kind) @@ -335,8 +337,8 @@ func TestBasicTimestamps(t *testing.T) { })) // The first start time is set in the constructor. - require.False(t, beforeNew.After(start1)) - require.False(t, afterNew.Before(start1)) + require.True(t, beforeNew.Before(start1)) + require.True(t, afterNew.After(start1)) for i := 0; i < 2; i++ { b.StartCollection() From ee1afc23f14e13fe69b39786dcc5df558e3f163d Mon Sep 17 00:00:00 2001 From: Punya Biswal Date: Tue, 23 Feb 2021 16:43:30 +0000 Subject: [PATCH 7/7] Keep time comparisons relaxed --- sdk/metric/aggregator/exact/exact_test.go | 14 +++++++++----- sdk/metric/processor/basic/basic_test.go | 16 ++++++++++------ 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/sdk/metric/aggregator/exact/exact_test.go b/sdk/metric/aggregator/exact/exact_test.go index d02f3905583..a02aa94edd5 100644 --- a/sdk/metric/aggregator/exact/exact_test.go +++ b/sdk/metric/aggregator/exact/exact_test.go @@ -33,6 +33,10 @@ type updateTest struct { count int } +func requireNotAfter(t *testing.T, t1, t2 time.Time) { + require.False(t, t1.After(t2), "expected %v ≤ %v", t1, t2) +} + func checkZero(t *testing.T, agg *Aggregator, desc *metric.Descriptor) { count, err := agg.Count() require.NoError(t, err) @@ -169,7 +173,7 @@ func (mt *mergeTest) run(t *testing.T, profile aggregatortest.Profile) { received.Append(s.Number) if i > 0 { - require.True(t, pts[i-1].Time.Before(pts[i].Time)) + requireNotAfter(t, pts[i-1].Time, pts[i].Time) } } @@ -296,11 +300,11 @@ func TestExactFloat64(t *testing.T) { for i := 0; i < len(po); i++ { require.Equal(t, all.Points()[i], po[i].Number, "Wrong point at position %d", i) if i > 0 { - require.True(t, po[i-1].Time.Before(po[i].Time)) + requireNotAfter(t, po[i-1].Time, po[i].Time) } } - require.True(t, po[0].Time.After(startTime)) - require.True(t, po[len(po)-1].Time.Before(endTime)) + requireNotAfter(t, startTime, po[0].Time) + requireNotAfter(t, po[len(po)-1].Time, endTime) } func TestSynchronizedMoveReset(t *testing.T) { @@ -352,7 +356,7 @@ func TestMergeBehavior(t *testing.T) { received.Append(s.Number) if i > 0 { - require.True(t, pts[i-1].Time.Before(pts[i].Time)) + requireNotAfter(t, pts[i-1].Time, pts[i].Time) } } diff --git a/sdk/metric/processor/basic/basic_test.go b/sdk/metric/processor/basic/basic_test.go index 1e8fc4e6c5a..599f6b8fb38 100644 --- a/sdk/metric/processor/basic/basic_test.go +++ b/sdk/metric/processor/basic/basic_test.go @@ -37,6 +37,10 @@ import ( "go.opentelemetry.io/otel/sdk/resource" ) +func requireNotAfter(t *testing.T, t1, t2 time.Time) { + require.False(t, t1.After(t2), "expected %v ≤ %v", t1, t2) +} + // TestProcessor tests all the non-error paths in this package. func TestProcessor(t *testing.T) { type exportCase struct { @@ -337,8 +341,8 @@ func TestBasicTimestamps(t *testing.T) { })) // The first start time is set in the constructor. - require.True(t, beforeNew.Before(start1)) - require.True(t, afterNew.After(start1)) + requireNotAfter(t, beforeNew, start1) + requireNotAfter(t, start1, afterNew) for i := 0; i < 2; i++ { b.StartCollection() @@ -355,8 +359,8 @@ func TestBasicTimestamps(t *testing.T) { // Subsequent intervals have their start and end aligned. require.Equal(t, start2, end1) - require.True(t, start1.Before(end1)) - require.True(t, start2.Before(end2)) + requireNotAfter(t, start1, end1) + requireNotAfter(t, start2, end2) start1 = start2 end1 = end2 @@ -516,6 +520,6 @@ func TestSumObserverEndToEnd(t *testing.T) { require.Equal(t, startTime[0], startTime[1]) require.Equal(t, startTime[0], startTime[2]) - require.True(t, endTime[0].Before(endTime[1])) - require.True(t, endTime[1].Before(endTime[2])) + requireNotAfter(t, endTime[0], endTime[1]) + requireNotAfter(t, endTime[1], endTime[2]) }