Instrument runs service DB repository layer with Prometheus metrics#7606
Instrument runs service DB repository layer with Prometheus metrics#7606youdie006 wants to merge 1 commit into
Conversation
Add a dbMetrics helper recording per-operation call/error counters and a latency stopwatch from a promutils.Scope sub-scope, mirroring the existing flyte metrics idiom. Thread a Scope into the action/task/project/trigger repo constructors and wrap each DB op via a DRY observe() helper. Metrics are labeled by operation name only; a nil scope is a no-op. Signed-off-by: youdie006 <youdie006@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9123e5d09
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| repo, err := repository.NewRepository(sc.DB, cfg.Database) | ||
| repo, err := repository.NewRepository(sc.DB, cfg.Database, sc.Scope) |
There was a problem hiding this comment.
In the standalone runs-service path, flytestdlib/app.App creates SetupContext without a scope and runs/cmd/main.go never sets sc.Scope before calling runs.Setup; because newDBMetrics(nil) is a no-op, passing sc.Scope here disables all of the newly added repository DB metrics for that binary. The manager path does set sc.Scope, so this only affects standalone deployments; initialize a real scope or provide a fallback before constructing the repos.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR instruments the runs service DB repository layer with Prometheus metrics by threading a promutils.Scope into repository constructors and wrapping each DB operation with a shared observation helper that records call count, error count, and latency per operation (low-cardinality, operation-name label only).
Changes:
- Add a reusable
dbMetricshelper (newDBMetrics+observe) that registers counters and a stopwatch under aScopesub-scope and records per-operation metrics. - Thread a
promutils.Scopethrough repository constructors (and setup wiring) and wrap repository DB operations viametrics.observe(...). - Add unit/integration-style tests validating nil-scope no-op behavior and that real repository operations emit the expected metrics.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| runs/test/api/setup_test.go | Updates repository construction to pass a nil metrics scope in tests. |
| runs/setup.go | Wires sc.Scope into repository creation and constructs a single project repo instance with a dedicated sub-scope to avoid duplicate metric registration. |
| runs/service/project_service_test.go | Updates project repo construction to pass a nil metrics scope in tests. |
| runs/repository/repository.go | Extends NewRepository to accept a promutils.Scope and creates per-repo sub-scopes for metrics isolation. |
| runs/repository/impl/metrics.go | Introduces dbMetrics (calls/errors counters + latency stopwatch) and an observe wrapper for DB ops. |
| runs/repository/impl/metrics_test.go | Adds tests covering nil-scope no-op and metrics emission on success/error, including an end-to-end CreateProject check. |
| runs/repository/impl/action.go | Adds metrics wiring to the action repo and wraps DB operations with metrics.observe(...). |
| runs/repository/impl/action_test.go | Updates action repo construction to pass a nil metrics scope in tests. |
| runs/repository/impl/task.go | Adds metrics wiring to the task repo and wraps DB operations with metrics.observe(...). |
| runs/repository/impl/task_test.go | Updates task repo construction to pass a nil metrics scope in tests. |
| runs/repository/impl/trigger.go | Adds metrics wiring to the trigger repo and wraps DB operations with metrics.observe(...). |
| runs/repository/impl/project.go | Adds metrics wiring to the project repo and wraps DB operations with metrics.observe(...). |
| runs/repository/impl/project_test.go | Updates project repo construction to pass a nil metrics scope in tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
We shouldn't be manually instrumenting every single DB call. This should be provided through some PG middleware registered as a hook. |
Summary
Implements #7448 — instruments the flyte2 runs service DB repository layer with Prometheus metrics.
Adds a
dbMetricshelper (runs/repository/impl/metrics.go) recording, per DB operation, a calls counter, an errors counter, and a latency stopwatch, built from apromutils.Scopesub-scope — mirroring flyte's existing metrics idiom (aMetricsstruct from apromutils.Scopevia anewMetrics(scope)constructor). Apromutils.Scopeis threaded into the action / task / project / trigger repo constructors (each gets its own named sub-scope), and every DB op is wrapped through a DRYobserve(ctx, operation, fn)helper. Metrics are labeled by operation name only (low cardinality, never row/project/user values). A nil scope yields a no-opdbMetrics, so constructors stay usable in unit tests.Test plan
runs/repository/impl/metrics_test.go(TDD red→green): nil-scope no-op, success/error observation, operation-only labeling, plus an end-to-endCreateProjecttest against the embedded postgres DB asserting the call counter, latency, and error counter.go build ./...exit 0,gofmt/go vetclean on touched files,go test ./runs/repository/impl/ok.Fixes #7448