flyteorg / flyteorg/flyte

[flyte2] Add /metrics endpoint and initialize metrics Scope in the app framework

Open
#7,446 3 comments 0 reactions 1 assignee Claimed by @aminehd View on GitHub
flyte2 good first issue
Dominant language
Go
Stars
7.5k
Forks
886
Avg merge
1d 12h
Merged PRs (30d)
120

Description

> Part of #7445 — **this is the foundation issue; the other sub-tasks depend on it.**

## Summary

Add a Prometheus `/metrics` HTTP endpoint to the v2 `app` framework and initialize the metrics `Scope` on the `SetupContext`, so every service built on it (runs service first) can expose and emit metrics.

## Why

**The real gap is that no `/metrics` endpoint is mounted anywhere.** `app.App.serve()` only serves `/healthz` and `/readyz` (`flytestdlib/app/app.go:94-109`). The v1 pattern using `promhttp.Handler()` lives in `flytestdlib/profutils/server.go:118` but was never wired into the v2 framework. So even though metrics are registered into the default Prometheus registry (storage sub-scope, the actions bloom-filter sub-scope, etc.), nothing serves them — a scrape returns nothing useful.

Secondary: the `app` framework's `serve()` never assigns `sc.Scope` (`flytestdlib/app/context.go:62-63`). The **unified manager** sets it (`manager/cmd/main.go:75`, `sc.Scope = promutils.NewScope("flyte")`), so services run via the manager get a non-nil scope. But the **standalone** binaries (`runs/cmd/main.go`, `actions/cmd/main.go`) do not set it, so anything reading `sc.Scope` there gets a nil interface. Initializing it at the framework level removes that footgun and means standalone binaries also get a working scope.

## What to do

In `flytestdlib/app/app.go`, inside `Run()` where the HTTP server is set up (the `if sc.Port > 0 {` block, around lines 92-109):

1. Initialize the metrics scope **before** `a.Setup(...)` is called, so services can use `sc.Scope` during setup (tasks #2–#4 register metrics in `runs/setup.go`):
```go
sc.Scope = promutils.NewScope(a.Name) // promutils.NewScope is defined in flytestdlib/promutils/scope.go:540
```
Pick a sensible, sanitized scope name from the app name (`a.Name`, e.g. `"runs-service"`). Note the unified manager already sets `sc.Scope = promutils.NewScope("flyte")` inside its own `setup()` (`manager/cmd/main.go:75`), which runs after the framework default and overrides it — that's fine. Consider only setting the framework default when `sc.Scope == nil` to make the precedence explicit.
2. Register the Prometheus handler on the mux next to `/healthz` and `/readyz`:
```go
sc.Mux.Handle("/metrics", promhttp.Handler())
```
Import `"github.com/prometheus/client_golang/prometheus/promhttp"`.

## Acceptance criteria

- [ ] Building and running any `app`-based service (e.g. runs service) exposes `GET /metrics` returning Prometheus text format (HTTP 200), including default Go/process collectors.
- [ ] `sc.Scope` is non-nil during `Setup` and after, and is the same scope used to register metrics.
- [ ] `/healthz` and `/readyz` continue to work unchanged.
- [ ] A unit test in `flytestdlib/app/` asserts that `/metrics` returns 200 and that `sc.Scope` is initialized.

## Pointers

- `flytestdlib/app/app.go` — `Run()` and the HTTP server block.
- `flytestdlib/app/context.go:62-63` — the existing `Scope` field.
- `flytestdlib/promutils/scope.go:540` — `NewScope(name string) Scope`.
- `flytestdlib/profutils/server.go:118` — reference for how `/metrics` was served in v1.

## Notes for contributors

- Keep the change minimal and framework-level; **do not** instrument individual services here (that's tasks #2–#4).
- Be careful registering metrics only once to avoid Prometheus duplicate-registration panics (use a single shared scope; `NewScope` + the default registry).

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.