Skip to content

collector: shared registry for collector enable/disable state (PoC) - #429

Draft
nicolastakashi wants to merge 1 commit into
prometheus:masterfrom
nicolastakashi:poc/collector-registry
Draft

collector: shared registry for collector enable/disable state (PoC)#429
nicolastakashi wants to merge 1 commit into
prometheus:masterfrom
nicolastakashi:poc/collector-registry

Conversation

@nicolastakashi

Copy link
Copy Markdown
Contributor

Draft / proof of concept — opening this to discuss the idea, not to merge as-is.

This came out of a review thread on postgres_exporter#1361, where a hand-maintained table of collectors and their default states prompted the question: could exporter-toolkit own collector enable/disable state, and could documentation be generated from it?

The duplication

node_exporter/collector/collector.go and postgres_exporter/collector/collector.go are near-verbatim copies of each other: factories, collectorState, forcedCollectors, initiatedCollectors, registerCollector, collectorFlagAction, and the filter-and-instantiate loop. postgres_exporter has a third partial copy in probe.go, whose // TODO: Handle filters node_exporter's copy had already solved — the kind of drift that duplication produces.

None of that depends on what a collector does, which is what makes it shareable.

What this adds

Registry[C, Cfg] is generic over the exporter's collector interface and its factory config, so no exporter changes its Collector type:

r.Register(Descriptor{Name, Help, DefaultEnabled, Requires}, factory)
r.AddFlags(app)              // --collector.X / --no-collector.X
r.DisableDefaults()          // preserves explicitly-named collectors
r.IsEnabled(name)
r.Build(cfgFor, filters...)  // lazy + cached, backs collect[]

Plus the introspection this enables, which is the part that isn't just deduplication:

  • Report() / WriteJSON() — one schema, same across exporters, for docs pipelines and tooling.
  • WriteMarkdown() — the default table rendering, for mdox or a CI diff check.
  • EnabledMetrics()<namespace>_collector_enabled{collector="..."}. A flag answers "what's enabled" for whoever can reach the process; a metric answers it for a fleet, and lets you alert on an exporter that shipped with a collector unintentionally off.
  • ScrapeReporter — the per-collector duration/success metrics both exporters define identically. Collect loops stay in the exporters, since only they know how to call their own Update.

Prototyped against two exporters

Both wired up locally, building and passing their suites, keeping registerCollector's signature so none of the ~60 node_exporter or ~26 postgres_exporter collector files changed:

lines notes
node_exporter −93 / +56 flag semantics verified identical, incl. --collector.disable-defaults interaction with explicit flags
postgres_exporter −115 / +111 includes new flags and metadata; removes the probe.go copy

Both net negative while gaining --collector.list and the enabled metric. The postgres_exporter side is at prometheus-community/postgres_exporter#TBD.

The generated postgres_exporter table reproduces all 26 rows of the hand-written one in #1361, defaults included.

Open questions

  • Is the generic-over-Cfg signature acceptable, or is it worth constraining? Build takes func(name string) Cfg rather than a plain Cfg because both exporters specialize per collector (logger.With("collector", name)); StaticConfig covers the simple case.
  • Should WriteMarkdown live here at all, or should the toolkit only emit JSON and leave rendering to each exporter's docs pipeline?
  • Naming: collector collides with the exporters' own collector packages, so both prototypes import it aliased.
  • Requires is free-form []string today. Worth making structured (extension vs. version), or is prose right for something purely informational?

go.mod only promotes client_golang and client_model from indirect to direct.

Exporters currently reimplement the same collector registry: the
--collector.<name> / --no-collector.<name> flag pair, a map of enabled
state, tracking which collectors the operator named explicitly, scrape
filters, and lazy instantiation with caching. node_exporter and
postgres_exporter carry near-identical copies of this code, and
postgres_exporter has a third partial copy in its probe handler.

None of that logic depends on what a collector actually does, so it can
be shared. Registry is generic over the exporter's collector interface
and its factory config, which lets each exporter keep its own Collector
type unchanged:

  node_exporter:     Update(ch) error
  postgres_exporter: Update(ctx, instance, ch) error

Sharing the state also makes it introspectable in one place. Descriptors
carry the metadata operators care about (default state, required
extensions or server versions), so Report/WriteJSON/WriteMarkdown can
answer "what collectors does this exporter have" in a form that is the
same across exporters -- usable for generating documentation tables that
cannot drift from the code. EnabledMetrics exposes the same state at
runtime as <namespace>_collector_enabled, so the question is also
answerable for a whole fleet rather than only by shelling into a host.

ScrapeReporter covers the per-collector duration and success metrics
that exporters define identically today. Collect loops stay in the
exporters, since only they know how to call their own Update.

Signed-off-by: Nicolas Takashi <nicolastakashicavalcante@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant