Skip to content

Commit e7727ea

Browse files
NickAngeFiery-Fenix
authored andcommitted
test: fix flaky TestTemporalityStartTimes by introducing a mockClock with added Time (open-telemetry#39306)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes open-telemetry#39219 (comment) <!--Describe what testing was performed and which tests were added.--> #### Testing <!--Describe the documentation added.--> #### Documentation <!--Please delete paragraphs that you did not use before submitting.-->
1 parent 2b95b6d commit e7727ea

File tree

5 files changed

+85
-13
lines changed

5 files changed

+85
-13
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: enhancement
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: telemetrygen
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Fix flaky test TestTemporalityStartTimes
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [39219]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: []

cmd/telemetrygen/pkg/metrics/clock.go

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Copyright The OpenTelemetry Authors
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package metrics
5+
6+
import (
7+
"time"
8+
)
9+
10+
type Clock interface {
11+
Now() time.Time
12+
}
13+
14+
type realClock struct{}
15+
16+
func (c *realClock) Now() time.Time {
17+
return time.Now()
18+
}
19+
20+
type mockClock struct {
21+
now time.Time
22+
}
23+
24+
func (c *mockClock) Now() time.Time {
25+
c.now = c.now.Add(100 * time.Millisecond) // Add 100ms to the mock clock to avoid timestamp collisions
26+
return c.now
27+
}

cmd/telemetrygen/pkg/metrics/metrics.go

+1
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ func run(c *Config, expF exporterFunc, logger *zap.Logger) error {
7777
wg: &wg,
7878
logger: logger.With(zap.Int("worker", i)),
7979
index: i,
80+
clock: &realClock{},
8081
}
8182
exp, err := expF()
8283
if err != nil {

cmd/telemetrygen/pkg/metrics/worker.go

+7-5
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ type worker struct {
2929
wg *sync.WaitGroup // notify when done
3030
logger *zap.Logger // logger
3131
index int // worker index
32+
clock Clock // clock
3233
}
3334

3435
// We use a 15-element bounds slice for histograms below, so there must be 16 buckets here.
@@ -84,13 +85,14 @@ var histogramBucketSamples = []struct {
8485
func (w worker) simulateMetrics(res *resource.Resource, exporter sdkmetric.Exporter, signalAttrs []attribute.KeyValue) {
8586
limiter := rate.NewLimiter(w.limitPerSecond, 1)
8687

87-
startTime := time.Now()
88+
startTime := w.clock.Now()
8889

8990
var i int64
9091
for w.running.Load() {
9192
var metrics []metricdata.Metrics
93+
now := w.clock.Now()
9294
if w.aggregationTemporality.AsTemporality() == metricdata.DeltaTemporality {
93-
startTime = time.Now().Add(-1 * time.Second)
95+
startTime = now.Add(-1 * time.Second)
9496
}
9597
switch w.metricType {
9698
case MetricTypeGauge:
@@ -99,7 +101,7 @@ func (w worker) simulateMetrics(res *resource.Resource, exporter sdkmetric.Expor
99101
Data: metricdata.Gauge[int64]{
100102
DataPoints: []metricdata.DataPoint[int64]{
101103
{
102-
Time: time.Now(),
104+
Time: now,
103105
Value: i,
104106
Attributes: attribute.NewSet(signalAttrs...),
105107
Exemplars: w.exemplars,
@@ -116,7 +118,7 @@ func (w worker) simulateMetrics(res *resource.Resource, exporter sdkmetric.Expor
116118
DataPoints: []metricdata.DataPoint[int64]{
117119
{
118120
StartTime: startTime,
119-
Time: time.Now(),
121+
Time: now,
120122
Value: i,
121123
Attributes: attribute.NewSet(signalAttrs...),
122124
Exemplars: w.exemplars,
@@ -139,7 +141,7 @@ func (w worker) simulateMetrics(res *resource.Resource, exporter sdkmetric.Expor
139141
DataPoints: []metricdata.HistogramDataPoint[int64]{
140142
{
141143
StartTime: startTime,
142-
Time: time.Now(),
144+
Time: now,
143145
Attributes: attribute.NewSet(signalAttrs...),
144146
Exemplars: w.exemplars,
145147
Count: totalCount,

cmd/telemetrygen/pkg/metrics/worker_test.go

+23-8
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ package metrics
55

66
import (
77
"context"
8-
"runtime"
98
"sync"
109
"sync/atomic"
1110
"testing"
@@ -478,9 +477,6 @@ func configWithMultipleAttributes(metric MetricType, qty int) *Config {
478477
}
479478

480479
func TestTemporalityStartTimes(t *testing.T) {
481-
if runtime.GOOS == "windows" {
482-
t.Skip("http://github.com/open-telemetry/opentelemetry-collector-contrib/issues/39219")
483-
}
484480
tests := []struct {
485481
name string
486482
temporality AggregationTemporality
@@ -490,23 +486,31 @@ func TestTemporalityStartTimes(t *testing.T) {
490486
name: "Cumulative temporality keeps same start timestamp",
491487
temporality: AggregationTemporality(metricdata.CumulativeTemporality),
492488
checkTimes: func(t *testing.T, firstTime, secondTime time.Time) {
493-
assert.Equal(t, firstTime, secondTime,
494-
"cumulative metrics should have same start time")
489+
if !assert.Equal(t, firstTime, secondTime,
490+
"cumulative metrics should have same start time") {
491+
logTimestampDiff(t, firstTime, secondTime)
492+
}
495493
},
496494
},
497495
{
498496
name: "Delta temporality has different start timestamps",
499497
temporality: AggregationTemporality(metricdata.DeltaTemporality),
500498
checkTimes: func(t *testing.T, firstTime, secondTime time.Time) {
501-
assert.True(t, secondTime.After(firstTime),
502-
"delta metrics should have increasing start times")
499+
if !assert.True(t, secondTime.After(firstTime),
500+
"delta metrics should have increasing start times") {
501+
logTimestampDiff(t, firstTime, secondTime)
502+
}
503503
},
504504
},
505505
}
506506

507507
for _, tt := range tests {
508508
t.Run(tt.name, func(t *testing.T) {
509509
m := &mockExporter{}
510+
clock := &mockClock{
511+
now: time.Now(),
512+
}
513+
510514
running := &atomic.Bool{}
511515
running.Store(true)
512516

@@ -522,6 +526,7 @@ func TestTemporalityStartTimes(t *testing.T) {
522526
limitPerSecond: rate.Inf,
523527
logger: zap.NewNop(),
524528
wg: wg,
529+
clock: clock,
525530
}
526531

527532
w.simulateMetrics(resource.Default(), m, nil)
@@ -540,3 +545,13 @@ func TestTemporalityStartTimes(t *testing.T) {
540545
})
541546
}
542547
}
548+
549+
func logTimestampDiff(t *testing.T, firstTime, secondTime time.Time) {
550+
t.Logf("Timestamp debug logging:\n"+
551+
"First start time: %s\n"+
552+
"Second start time: %s\n"+
553+
"Difference: %v",
554+
firstTime.String(),
555+
secondTime.String(),
556+
secondTime.Sub(firstTime))
557+
}

0 commit comments

Comments
 (0)