Part of #7445. Small starter cleanup.
Summary
executor/setup.go constructs promutils.NewScope("executor") twice with the same base name. Dedupe it into a single shared scope to avoid confusion and the risk of duplicate-metric-registration panics.
Background
In executor/setup.go:
setup.go:112 — webhookPkg.Setup(ctx, kubeClient, wCfg, podNamespace, promutils.NewScope("executor"), mgr)
setup.go:124 — plugin.NewSetupContext(mgr, nil, nil, nil, nil, "TaskAction", promutils.NewScope("executor"))
Both create a scope with the identical base name "executor". promutils registers metrics on the default Prometheus registry via prometheus.Register(...) (flytestdlib/promutils/scope.go:269-419), which panics on duplicate registration. Today this is latent (the two consumers happen to register different metric names), but two independent scopes sharing a base name is fragile — if both ever register a like-named metric, the executor panics at startup.
What to do
- Construct the executor base scope once, e.g.:
executorScope := promutils.NewScope("executor")
- Pass distinct sub-scopes to each consumer so names can't collide, e.g.:
- webhook:
executorScope.NewSubScope("webhook")
- plugins:
executorScope.NewSubScope("plugin")
(Adjust names to match conventions; the storage and catalog scopes already use distinct names executor:storage / executor:catalog.)
Acceptance criteria
Pointers
executor/setup.go:112 and setup.go:124 — the two NewScope("executor") calls.
flytestdlib/promutils/scope.go — NewScope / NewSubScope and the prometheus.Register calls that panic on duplicates.
Notes for contributors
- Pure refactor — no behavior change beyond scope naming. Great first PR.
Summary
executor/setup.goconstructspromutils.NewScope("executor")twice with the same base name. Dedupe it into a single shared scope to avoid confusion and the risk of duplicate-metric-registration panics.Background
In
executor/setup.go:setup.go:112—webhookPkg.Setup(ctx, kubeClient, wCfg, podNamespace, promutils.NewScope("executor"), mgr)setup.go:124—plugin.NewSetupContext(mgr, nil, nil, nil, nil, "TaskAction", promutils.NewScope("executor"))Both create a scope with the identical base name
"executor".promutilsregisters metrics on the default Prometheus registry viaprometheus.Register(...)(flytestdlib/promutils/scope.go:269-419), which panics on duplicate registration. Today this is latent (the two consumers happen to register different metric names), but two independent scopes sharing a base name is fragile — if both ever register a like-named metric, the executor panics at startup.What to do
executorScope.NewSubScope("webhook")executorScope.NewSubScope("plugin")(Adjust names to match conventions; the storage and catalog scopes already use distinct names
executor:storage/executor:catalog.)Acceptance criteria
promutils.NewScope("executor")is constructed exactly once inexecutor/setup.go.Pointers
executor/setup.go:112andsetup.go:124— the twoNewScope("executor")calls.flytestdlib/promutils/scope.go—NewScope/NewSubScopeand theprometheus.Registercalls that panic on duplicates.Notes for contributors