flyteorg/flyte

[flyte2] Executor: dedupe the two promutils.NewScope("executor") constructions in setup.go

Open

Aperta il 29 mag 2026

Vedi su GitHub
 (1 commento) (0 reazioni) (0 assegnatari)Python (3705 star) (378 fork)batch import
flyte2good first issue

Descrizione

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:112webhookPkg.Setup(ctx, kubeClient, wCfg, podNamespace, promutils.NewScope("executor"), mgr)
  • setup.go:124plugin.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

  1. Construct the executor base scope once, e.g.:
    executorScope := promutils.NewScope("executor")
    
  2. 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

  • promutils.NewScope("executor") is constructed exactly once in executor/setup.go.
  • Webhook and plugin consumers receive distinct sub-scopes derived from it.
  • The executor still starts cleanly (no duplicate-registration panic) and existing tests pass.

Pointers

  • executor/setup.go:112 and setup.go:124 — the two NewScope("executor") calls.
  • flytestdlib/promutils/scope.goNewScope / NewSubScope and the prometheus.Register calls that panic on duplicates.

Notes for contributors

  • Pure refactor — no behavior change beyond scope naming. Great first PR.

Guida contributor