Skip to content

chore: add workqueue claim time meter #846

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

Open
adityachoudhari26 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

chore: add workqueue claim time meter #846

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
28 changes: 28 additions & 0 deletions apps/workspace-engine/pkg/reconcile/worker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,28 @@ import (
"time"

"github.com/charmbracelet/log"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric"
"workspace-engine/svc"
)

var meter = otel.Meter("workspace-engine/reconcile")

var claimWaitHistogram metric.Float64Histogram

func init() {
var err error
claimWaitHistogram, err = meter.Float64Histogram(
"reconcile.claim_wait_seconds",
metric.WithDescription("Time between an item becoming eligible (not_before) and being claimed"),
metric.WithUnit("s"),
)
if err != nil {
panic(err)
}
}
Comment on lines +16 to +30
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

In OpenTelemetry Go v1.38.0, if otel.Meter(...)andmeter.Float64Histogram(...)are called beforeotel.SetMeterProvider(...), do those instruments emit via the later provider or remain attached to the initial no-op provider?

💡 Result:

They will emit via the later provider (after the first otel.SetMeterProvider(...)).

In go.opentelemetry.io/otel v1.38.0, calling otel.Meter(...) before any global MeterProvider is registered returns a no-op meter temporarily, but when otel.SetMeterProvider(...) is called for the first time, that previously returned Meter and all instruments it created (e.g., Float64Histogram) are “recreated automatically” from the new provider—so subsequent recordings go through the newly set provider. Any measurements recorded before the provider was set are dropped. [1]

This matches the Metrics SDK requirement that updated configuration must apply to already-returned Meters (i.e., it must not matter whether a Meter was obtained before or after the change). [2]

Sources:
[1] otel.Meter / otel.GetMeterProvider docs (v1.38.0) (pkg.go.dev)
[2] OpenTelemetry Metrics SDK spec (configuration changes apply to already returned meters) (opentelemetry.io)

Citations:

Avoid panicking on histogram initialization failure.

Line 28 can crash the service for a telemetry setup error, which should not block the worker from starting. Either defer histogram initialization or gracefully handle the error.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/workspace-engine/pkg/reconcile/worker.go` around lines 16 - 30, The
init() currently panics if meter.Float64Histogram for
"reconcile.claim_wait_seconds" fails; replace the panic with a non-fatal
handling: log the error (e.g., using the package logger or log.Printf) and leave
claimWaitHistogram nil (or assign a no-op histogram if you have a helper), and
then update all places that use claimWaitHistogram to guard against nil before
recording (i.e., check claimWaitHistogram != nil in call sites) so telemetry
setup failures don't crash the worker (refer to init(), meter.Float64Histogram,
and claimWaitHistogram).


const (
defaultRetryBackoffCap = 5 * time.Minute
)
Expand Down Expand Up @@ -162,7 +181,16 @@ func (w *Worker) startItems(
sem chan struct{},
doneCh chan struct{},
) {
now := time.Now()
for _, item := range items {
claimWait := now.Sub(item.NotBefore).Seconds()
if claimWait < 0 {
claimWait = 0
}
claimWaitHistogram.Record(ctx, claimWait,
metric.WithAttributes(attribute.String("kind", item.Kind)),
)

if w.cfg.Hooks.OnClaimed != nil {
w.cfg.Hooks.OnClaimed(item)
}
Expand Down
Loading