refactor(controller): route metrics through observability/metrics Emitter#215
Merged
Conversation
xytan0056
changed the title
xytan0056
force-pushed
the
pr2-controller-metrics
branch
2 times, most recently
from
July 17, 2026 08:09
61b8436 to
fc94dd2
Compare
xytan0056
changed the title
xytan0056
force-pushed
the
pr2-controller-metrics
branch
from
July 17, 2026 09:12
fc94dd2 to
206444a
Compare
xytan0056
force-pushed
the
pr1-emitter-core
branch
from
July 17, 2026 17:15
72fb35c to
826487e
Compare
xytan0056
force-pushed
the
pr2-controller-metrics
branch
from
July 17, 2026 17:15
206444a to
d62bb17
Compare
xytan0056
changed the title
xytan0056
changed the title
xytan0056
force-pushed
the
pr1-emitter-core
branch
from
July 17, 2026 23:10
826487e to
fb00357
Compare
xytan0056
force-pushed
the
pr2-controller-metrics
branch
from
July 17, 2026 23:10
d62bb17 to
7a8d276
Compare
yushan8
reviewed
Jul 21, 2026
| // graph fetches, and comparison phases. These can run long (a graph fetch | ||
| // or compare may take tens of minutes), so the range extends to ~80m: | ||
| // exponential 1ms..~80m. | ||
| stepDurationBuckets = tally.MustMakeExponentialDurationBuckets(time.Millisecond, 3, 15) |
Contributor
There was a problem hiding this comment.
70-80 minutes sounds pretty extreme. Do need buckets that go that high? Maybe limit it to 30 minutes?
Contributor
Author
There was a problem hiding this comment.
a few extra buckets is no big deal. we don't lose visibility into tail cases. plus the long tails are still recorded. the granularity is not affected
yushan8
approved these changes
Jul 21, 2026
…tter Port the controller off tally.Scope to *metrics.Emitter using the Begin/Complete lifecycle helper per docs/observability/metrics.md. - Params.Scope tally.Scope -> Emitter *metrics.Emitter (optional, nil->Nop) - Each RPC bakes the repo tag into a per-request emitter, then Begin/Complete emit start + result-tagged finish (replacing calls/success/failure and the total_duration timers); no domain metrics struct or memoization - Sub-operations (get_graph, compare_target_graphs) get their own Begin/Complete lifecycle; compare_target_graphs flattens to its own op - Custom timers -> DurationHistogram on the repo-tagged emitter with explicit package-level buckets (controller/metrics.go) - emitFailureMetric(e, op, err) keeps the failure_type/reason axis - Add controller/metrics_test.go asserting the start/finish path shape
xytan0056
force-pushed
the
pr2-controller-metrics
branch
from
July 21, 2026 06:30
7a8d276 to
e77a1cb
Compare
2 tasks
xytan0056
added a commit
that referenced
this pull request
Jul 21, 2026
…tter (#227) ## Summary redo: #215 , which landed on the wrong branch - Ports the controller off raw `tally.Scope` counters/timers to `*metrics.Emitter` with the `Begin`/`Complete` lifecycle helper. - `Params` keeps `Scope tally.Scope`; `NewController` subscopes it to `controller` and builds its own emitter (nil → no-op). - Each RPC bakes the `repo` tag once, then `Begin`/`Complete` emit `start` + result-tagged `finish` (replacing `calls`/`success`/`failure` + `total_duration` timers). - Sub-ops `get_graph` and `compare_target_graphs` get their own lifecycle; `compare_target_graphs` flattens to its own op. - Other timers → `DurationHistogram` with package-level buckets (`controller/metrics.go`); `emitFailureMetric` keeps the `failure_type`/`failure_reason` axis. Replaces #215 (auto-closed when base branch `pr1-emitter-core` merged). ## Test plan - [x] `make build` passes - [x] `make test` — all 21 tests pass
xytan0056
added a commit
that referenced
this pull request
Jul 21, 2026
…mitter (#216) Ports `native_orchestrator`'s `GetTargetGraph` off raw `tally.Scope` to `*metrics.Emitter` with the `Begin`/`Complete` helper. - `Params` keeps `Scope tally.Scope`; the orchestrator self-subscopes to `orchestrator` and builds its own emitter (still forwards the scope to the graph runner). - `start` + result-tagged `finish` (replaces `calls`/`success`/`failure`). - Per-step duration histograms under `get_target_graph`: `lease_duration`, `checkout_duration`, `apply_requests_duration`, `cache_read_duration`, `compute_duration`, `cache_write_duration`. Stacked on #215.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports the controller off raw
tally.Scopecounters/timers to*metrics.Emitterwith theBegin/Completelifecycle helper.ParamskeepsScope tally.Scope;NewControllersubscopes it tocontrollerand builds its own emitter (nil → no-op). Same for every component — no injected emitter.repotag once, thenBegin/Completeemitstart+ result-taggedfinish(replacingcalls/success/failure+total_durationtimers).get_graphandcompare_target_graphsget their own lifecycle;compare_target_graphsflattens to its own op.DurationHistogramwith package-level buckets (controller/metrics.go);emitFailureMetrickeeps thefailure_type/failure_reasonaxis.