Skip to content

feat: add per-phase histogram metrics to engine worker pipeline - #21547

Merged
Segflow merged 3 commits into
mainfrom
meher/histogram-per-phase-timing
Apr 17, 2026
Merged

feat: add per-phase histogram metrics to engine worker pipeline#21547
Segflow merged 3 commits into
mainfrom
meher/histogram-per-phase-timing

Conversation

@Segflow

@Segflow Segflow commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

Adds native histogram metrics to instrument the three phases (read, compute, write) of the engine worker's drainPipeline loop, giving visibility into where time is spent during task execution.

Two granularity levels are introduced:

Per-pass metrics — one observation per loop iteration in drainPipeline:

  • loki_engine_worker_pass_compute_seconds — duration of a single pipeline.Read (compute) call
  • loki_engine_worker_pass_write_seconds — duration of sending a single record batch to sinks

Per-task metrics — one observation per drainPipeline invocation:

  • loki_engine_worker_task_read_seconds — time spent in pipeline.Open (read/init phase)
  • loki_engine_worker_task_compute_seconds — cumulative compute time across all passes
  • loki_engine_worker_task_write_seconds — cumulative write time across all passes

All histograms use native histogram buckets (factor 1.1, max 100 buckets, 1h min reset).

Which issue(s) this PR fixes:
N/A

Special notes for your reviewer:
N/A

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • Title matches the required conventional commits format, see here
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

Note

Low Risk
Low risk: adds Prometheus native-histogram observations around existing drainPipeline phases without changing task execution behavior.

Overview
Adds new Prometheus native histograms on the worker to track per-pass pipeline.Read (compute) and sink send (write) durations, plus per-task totals for pipeline.Open (read/init) and accumulated compute/write time across the drain loop.

Updates thread.drainPipeline to time and record these phase metrics (and reuse the measured write duration for existing xcap.TaskSendDuration), and adjusts TestThread_drainPipeline to initialize thread.Metrics.

Reviewed by Cursor Bugbot for commit b990a04. Bugbot is set up for automated code reviews on this repo. Configure here.

@Segflow
Segflow requested a review from a team as a code owner April 14, 2026 08:29

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Final EOF read's compute time is silently discarded
    • The drain loop now records and accumulates compute duration for successful reads and the EOF read before breaking, so task compute timing includes the final pass.

Create PR

Or push these changes by commenting:

@cursor push d7a30c883a
Preview (d7a30c883a)
diff --git a/pkg/engine/internal/worker/thread.go b/pkg/engine/internal/worker/thread.go
--- a/pkg/engine/internal/worker/thread.go
+++ b/pkg/engine/internal/worker/thread.go
@@ -347,13 +347,15 @@
 		startCompute := time.Now()
 		rec, err := pipeline.Read(ctx)
 		computeDuration := time.Since(startCompute)
+		if err == nil || errors.Is(err, executor.EOF) {
+			t.Metrics.passComputeSeconds.Observe(computeDuration.Seconds())
+			totalComputeTime += computeDuration
+		}
 		if err != nil && errors.Is(err, executor.EOF) {
 			break
 		} else if err != nil {
 			return totalRows, err
 		}
-		t.Metrics.passComputeSeconds.Observe(computeDuration.Seconds())
-		totalComputeTime += computeDuration
 
 		region.Record(xcap.TaskDrainRecordsReceived.Observe(1))
 		totalRows += int(rec.NumRows())

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit 64bb7ba. Configure here.

Comment thread pkg/engine/internal/worker/thread.go

@rfratto rfratto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, though Cursor is right here that we're not observing the time it takes to compute the final record where EOF is expected.

@cla-assistant

cla-assistant Bot commented Apr 15, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@Segflow
Segflow force-pushed the meher/histogram-per-phase-timing branch from fc0fc17 to b990a04 Compare April 17, 2026 10:45
@grafana grafana deleted a comment from cla-assistant Bot Apr 17, 2026
@Segflow
Segflow merged commit c0a243b into main Apr 17, 2026
89 checks passed
@Segflow
Segflow deleted the meher/histogram-per-phase-timing branch April 17, 2026 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2 participants