collectors: consolidate config and introduce Runtime by kgeckhart · Pull Request #500 · prometheus-community/stackdriver_exporter

kgeckhart · 2026-05-06T18:53:46Z

The exporter's config + assembly logic is currently spread across config/ (mixing pure config with OTel-bridge plumbing — Option, AllOptions, OTelComponentDefaults), main() (project resolution, monitoring service creation), and the HTTP handler (collector cache). There's no single entry point for building collectors from a config.

This PR cleans up the layering:

config/: pure value-type package — Config, defaults, NewConfigWithDefaults, Validate. No GCP or collectors deps.
collectors.NewRuntime: single assembly entry point. Returns a *Runtime with API Collectors / CollectorsForPrefixes(prefix) / WithCache.
utils/ deleted: helpers now live unexported in collectors/ next to their callers.
Filter-path scrape errors return HTTP 500 instead of os.Exit(1) — one project's collector-build failure no longer crashes the process.

The public Go API surface is reduced. The only known external consumer (grafana/alloy and its forks) is unaffected — verified against their gcp_exporter.go and self_pruning_store.go.

ArthurSens

LGTM, but do you mind opening a PR in https://github.com/prometheus/prometheus-opentelemetry-collector with the decoder implementation?

kgeckhart · 2026-05-06T21:06:46Z

I'm drafting it while I work through the end-to-end semantics. It was all working cleanly but the decoder on the other side ended up being a bit of a mess.

ArthurSens

I noticed you chose to perform the Config validation inside NewRuntime rather than in a separate func (c *config) Validate() function. Was that intentional?

I was also a bit confused by the removal of the config package, but after reading your comment here prometheus/prometheus-opentelemetry-collector#13 (comment), it's clear this was a conscious decision.

I'm not strongly opinionated about it. Since the config will always be used by the package that generates the Prometheus Registry, it sounds ok to keep both in the same package. I don't see anything bad in keeping them separate, either.

kgeckhart · 2026-05-13T13:43:38Z

I'm not strongly opinionated about it. Since the config will always be used by the package that generates the Prometheus Registry, it sounds ok to keep both in the same package. I don't see anything bad in keeping them separate, either.

I think I'm on board with keeping them separated at this point. It can leave us open to some interesting things where we could have a consumer that only needs the config package to do automated config rendering for high level use cases like "collect metrics from GCP VMs". Most of the awkwardness I found during this further refactor was caused by the various functions living in different places. Resolving some of your comments and taking a closer look at where the functions live should clear that part up.

Signed-off-by: Kyle Eckhart <kgeckhart@users.noreply.github.com>

ArthurSens · 2026-05-13T18:39:59Z

+func normalizeMetricName(metricName string) string {
+	var parts []string
+	for _, word := range camelcase.Split(metricName) {
+		safe := strings.Trim(safeNameRE.ReplaceAllLiteralString(word, "_"), "_")
+		lower := strings.TrimSpace(strings.ToLower(safe))
+		if lower != "" {
+			parts = append(parts, lower)
+		}
+	}
+	return strings.Join(parts, "_")


I know you're just moving a function from one place to another, so completely out of scope for this PR, but... in prometheus/otlptranslator we have an API that does the same thing and doesn't depend on regex matching. It's probably a lot faster :)

ArthurSens

Only one suggestion, otherwise LGTM!

ArthurSens · 2026-05-13T18:51:26Z

+	// Validate up front so CLI users get a clean error before any GCP client
+	// setup. NewRuntime also calls Validate so embedded callers don't have to.
+	if err := cfg.Validate(); err != nil {
+		logger.Error("invalid configuration", "err", err)
+		os.Exit(1)
+	}
+
+	runtime, err := collectors.NewRuntime(ctx, logger, cfg, delta.NewInMemoryCounterStore, delta.NewInMemoryHistogramStore)


You're calling NewRuntime, which also calls Validate(), right after the Validate() call here. I think we should choose one. Either call Validate outside of NewRuntime, and therefore delete the Validate call from there, or remove this call of Validate from stackdriver_exporter.go

WDYT about 7f7bd26? I realized I have the same double validate dance in the receiver side. I mostly wanted to protect from unvalidated configs being sent through. Someone can still mutate after the fact but then they are intentionally breaking it vs unknowingly.

Signed-off-by: Kyle Eckhart <kgeckhart@users.noreply.github.com>

ArthurSens

LGTM after the doc fixes

ArthurSens

just one nit actually

ArthurSens · 2026-05-13T19:14:47Z

+// to derive a sibling that does.
+func NewRuntime(ctx context.Context, logger *slog.Logger, cfg *config.Config, counterFactory CounterStoreFactory, histogramFactory HistogramStoreFactory) (*Runtime, error) {
+	if !cfg.Validated() {
+		return nil, fmt.Errorf("config has not been validated; call cfg.Validate before NewRuntime")


Maybe panic here? There's no action item for the end-user if we ever hit this. It is clearly a bug on our side

I'm not strongly opinionated though, I know panics are very very bad

🤔 fair point, hmm the CLI is going to end in error either way. An embedder might not want to crash if the component fails even for a programming error like this. I think I like an error since it's less forceful. I know in some early refactorings of other exporters the first thing we often did was expose errors vs panics especially for use cases that involve multi-tenancy panic'ing can be really impactful.

kgeckhart requested review from ArthurSens and SuperQ May 6, 2026 18:53

ArthurSens approved these changes May 6, 2026

View reviewed changes

kgeckhart marked this pull request as draft May 6, 2026 21:06

ArthurSens self-requested a review May 7, 2026 19:28

kgeckhart force-pushed the kgeckhart/add-runtime-config-validate branch from 6909981 to d2cd4e8 Compare May 11, 2026 18:59

kgeckhart changed the title ~~config: add Validate to RuntimeConfig, remove OTel-specific fields from Option~~ collectors: consolidate config and introduce Runtime May 11, 2026

kgeckhart mentioned this pull request May 11, 2026

receivers/stackdriver: decode YAML directly into collectors.Config prometheus/prometheus-opentelemetry-collector#12

Draft

kgeckhart marked this pull request as ready for review May 11, 2026 19:38

ArthurSens reviewed May 12, 2026

View reviewed changes

Comment thread collectors/config.go Outdated

Comment thread collectors/config.go Outdated

Comment thread stackdriver_exporter.go Outdated

ArthurSens reviewed May 12, 2026

View reviewed changes

Comment thread collectors/config.go Outdated

Comment thread collectors/config.go Outdated

collectors: consolidate config and introduce Runtime

54607be

Signed-off-by: Kyle Eckhart <kgeckhart@users.noreply.github.com>

kgeckhart force-pushed the kgeckhart/add-runtime-config-validate branch from d2cd4e8 to 54607be Compare May 13, 2026 18:30

ArthurSens reviewed May 13, 2026

View reviewed changes

config: gate NewRuntime on Validated() instead of re-running Validate

7f7bd26

Signed-off-by: Kyle Eckhart <kgeckhart@users.noreply.github.com>

ArthurSens reviewed May 13, 2026

View reviewed changes

Comment thread collectors/runtime.go Outdated

config, collectors: trim package-leaking detail from doc comments

00b90d3

Signed-off-by: Kyle Eckhart <kgeckhart@users.noreply.github.com>

ArthurSens approved these changes May 13, 2026

View reviewed changes

ArthurSens reviewed May 13, 2026

View reviewed changes

ArthurSens linked an issue May 22, 2026 that may be closed by this pull request

Embed Stackdriver exporter prometheus/opentelemetry-collector-bridge#37

Open

ArthurSens removed a link to an issue May 22, 2026

Embed Stackdriver exporter prometheus/opentelemetry-collector-bridge#37

Open

ArthurSens linked an issue May 22, 2026 that may be closed by this pull request

Refactor Stackdriver exporter prometheus/opentelemetry-collector-bridge#46

Open

Conversation

kgeckhart commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ArthurSens left a comment

Choose a reason for hiding this comment

Uh oh!

kgeckhart commented May 6, 2026

Uh oh!

ArthurSens left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kgeckhart commented May 13, 2026

Uh oh!

ArthurSens May 13, 2026

Choose a reason for hiding this comment

Uh oh!

ArthurSens left a comment

Choose a reason for hiding this comment

Uh oh!

ArthurSens May 13, 2026

Choose a reason for hiding this comment

Uh oh!

kgeckhart May 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ArthurSens left a comment

Choose a reason for hiding this comment

Uh oh!

ArthurSens left a comment

Choose a reason for hiding this comment

Uh oh!

ArthurSens May 13, 2026

Choose a reason for hiding this comment

Uh oh!

ArthurSens May 13, 2026

Choose a reason for hiding this comment

Uh oh!

kgeckhart May 13, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kgeckhart commented May 6, 2026 •

edited

Loading