Skip to content

Commit ee16898

Browse files
authored
fix: Properly handle relational tables metrics (#1788)
#### Summary Follow up to table OTEL metrics support. For relational tables `OtelStartTime` and `OtelEndTime` are called multiple times (per parent resource), so we were reporting the start and end time incorrectly (adding up all the times). This PR fixes it so only the first start time is reported, and the end time gets updated to the latest value. Another possible fix to the end time is to set it to whenever the parent table finishes syncing, but I don't think that's very useful. ---
1 parent 2cf9a71 commit ee16898

1 file changed

Lines changed: 28 additions & 7 deletions

File tree

scheduler/metrics.go

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package scheduler
22

33
import (
44
"context"
5+
"sync"
56
"sync/atomic"
67
"time"
78

@@ -17,12 +18,16 @@ type Metrics struct {
1718
}
1819

1920
type OtelMeters struct {
20-
resources metric.Int64Counter
21-
errors metric.Int64Counter
22-
panics metric.Int64Counter
23-
startTime metric.Int64Counter
24-
endTime metric.Int64Counter
25-
attributes []attribute.KeyValue
21+
resources metric.Int64Counter
22+
errors metric.Int64Counter
23+
panics metric.Int64Counter
24+
startTime metric.Int64Counter
25+
started bool
26+
startedLock sync.Mutex
27+
endTime metric.Int64Counter
28+
previousEndTime int64
29+
previousEndTimeLock sync.Mutex
30+
attributes []attribute.KeyValue
2631
}
2732

2833
type TableClientMetrics struct {
@@ -234,6 +239,13 @@ func (m *TableClientMetrics) OtelStartTime(ctx context.Context, start time.Time)
234239
return
235240
}
236241

242+
// If we have already started, don't start again. This can happen for relational tables that are resolved multiple times (per parent resource)
243+
m.otelMeters.startedLock.Lock()
244+
defer m.otelMeters.startedLock.Unlock()
245+
if m.otelMeters.started {
246+
return
247+
}
248+
m.otelMeters.started = true
237249
m.otelMeters.startTime.Add(ctx, start.UnixNano(), metric.WithAttributes(m.otelMeters.attributes...))
238250
}
239251

@@ -242,5 +254,14 @@ func (m *TableClientMetrics) OtelEndTime(ctx context.Context, end time.Time) {
242254
return
243255
}
244256

245-
m.otelMeters.endTime.Add(ctx, end.UnixNano(), metric.WithAttributes(m.otelMeters.attributes...))
257+
m.otelMeters.previousEndTimeLock.Lock()
258+
defer m.otelMeters.previousEndTimeLock.Unlock()
259+
val := end.UnixNano()
260+
// If we got another end time to report, use the latest value. This can happen for relational tables that are resolved multiple times (per parent resource)
261+
if m.otelMeters.previousEndTime != 0 {
262+
m.otelMeters.endTime.Add(ctx, val-m.otelMeters.previousEndTime, metric.WithAttributes(m.otelMeters.attributes...))
263+
} else {
264+
m.otelMeters.endTime.Add(ctx, val, metric.WithAttributes(m.otelMeters.attributes...))
265+
}
266+
m.otelMeters.previousEndTime = val
246267
}

0 commit comments

Comments
 (0)