From c1c4631250d417c70599b60625afe230ea9957c9 Mon Sep 17 00:00:00 2001 From: erezrokah Date: Mon, 8 Jul 2024 18:58:53 +0100 Subject: [PATCH 1/2] fix: Properly handle relational tables metrics --- scheduler/metrics.go | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/scheduler/metrics.go b/scheduler/metrics.go index 3aa18e3c3a..b5f9095480 100644 --- a/scheduler/metrics.go +++ b/scheduler/metrics.go @@ -2,6 +2,7 @@ package scheduler import ( "context" + "sync" "sync/atomic" "time" @@ -17,12 +18,16 @@ type Metrics struct { } type OtelMeters struct { - resources metric.Int64Counter - errors metric.Int64Counter - panics metric.Int64Counter - startTime metric.Int64Counter - endTime metric.Int64Counter - attributes []attribute.KeyValue + resources metric.Int64Counter + errors metric.Int64Counter + panics metric.Int64Counter + startTime metric.Int64Counter + started bool + startedLock sync.Mutex + endTime metric.Int64Counter + previousEndTime int64 + previousEndTimeLock sync.Mutex + attributes []attribute.KeyValue } type TableClientMetrics struct { @@ -234,6 +239,13 @@ func (m *TableClientMetrics) OtelStartTime(ctx context.Context, start time.Time) return } + // If we have already started, don't start again. This can happen for relational tables that are resolved multiple times (per parent resource) + m.otelMeters.startedLock.Lock() + defer m.otelMeters.startedLock.Unlock() + if m.otelMeters.started { + return + } + m.otelMeters.started = true m.otelMeters.startTime.Add(ctx, start.UnixNano(), metric.WithAttributes(m.otelMeters.attributes...)) } @@ -242,5 +254,15 @@ func (m *TableClientMetrics) OtelEndTime(ctx context.Context, end time.Time) { return } - m.otelMeters.endTime.Add(ctx, end.UnixNano(), metric.WithAttributes(m.otelMeters.attributes...)) + m.otelMeters.previousEndTimeLock.Lock() + defer m.otelMeters.previousEndTimeLock.Unlock() + val := end.UnixNano() + // 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) + if m.otelMeters.previousEndTime != 0 { + m.otelMeters.endTime.Add(ctx, val-m.otelMeters.previousEndTime, metric.WithAttributes(m.otelMeters.attributes...)) + } else { + m.otelMeters.endTime.Add(ctx, val, metric.WithAttributes(m.otelMeters.attributes...)) + } + m.otelMeters.previousEndTime = val + } From 592c47e715a007701aec854505ef83353a4592b1 Mon Sep 17 00:00:00 2001 From: erezrokah Date: Tue, 9 Jul 2024 09:05:14 +0100 Subject: [PATCH 2/2] style: Fix lint --- scheduler/metrics.go | 1 - 1 file changed, 1 deletion(-) diff --git a/scheduler/metrics.go b/scheduler/metrics.go index b5f9095480..54d34873cb 100644 --- a/scheduler/metrics.go +++ b/scheduler/metrics.go @@ -264,5 +264,4 @@ func (m *TableClientMetrics) OtelEndTime(ctx context.Context, end time.Time) { m.otelMeters.endTime.Add(ctx, val, metric.WithAttributes(m.otelMeters.attributes...)) } m.otelMeters.previousEndTime = val - }