Background
While fact-checking the docs in #189 against the source, we found the design-internals doc described behavior the code does not implement. The doc has been corrected to match the code (in #189), but the underlying API ergonomics question is worth a decision.
Current behavior (verified against source)
observability.Init (observability/observability.go:102) calls only cfg.validate().Config.validate() (observability/config.go:151) does not mutate the config — its own comment says callers "should compose with DefaultConfig".DefaultConfig() (observability/config.go:134) sets only EnableRuntimeMetrics=true, EnableProcessMetrics=true, MetricsPushInterval=30s. It does not set MetricsMode.- A zero/empty
MetricsMode is therefore rejected: validate() returns invalid MetricsMode (config.go:158), and Init's switch has a default error case (observability.go:150).
There is no applyDefaults function and no defaults.go file — the old doc invented both.
Why it's a papercut
DefaultConfig() reads as "supply ServiceName and MetricsMode and you're good." But:
cfg := observability.DefaultConfig()
cfg.ServiceName = "tasks-svc"
// forgets MetricsMode
obs, err := observability.Init(ctx, cfg) // -> error: invalid MetricsMode ""
fails at runtime for what looks like a reasonable default path. Prometheus is the obvious default mode.
Decision needed
Option A — make code match the (old) doc intent: have DefaultConfig() set MetricsMode = MetricsModePrometheus (and/or have Init default a zero value). Friendlier; one fewer required field.
Option B — keep MetricsMode required: current behavior; explicit over implicit. Docs already corrected to state this.
If A: implement in DefaultConfig() (no mutation in Init), add a test, re-document MetricsMode as defaulted. If B: no code change — close this issue; the #189 doc fix already reflects it.
Refs: #189 (doc correction).
Background
While fact-checking the docs in #189 against the source, we found the design-internals doc described behavior the code does not implement. The doc has been corrected to match the code (in #189), but the underlying API ergonomics question is worth a decision.
Current behavior (verified against source)
observability.Init(observability/observability.go:102) calls onlycfg.validate().Config.validate()(observability/config.go:151) does not mutate the config — its own comment says callers "should compose withDefaultConfig".DefaultConfig()(observability/config.go:134) sets onlyEnableRuntimeMetrics=true,EnableProcessMetrics=true,MetricsPushInterval=30s. It does not setMetricsMode.MetricsModeis therefore rejected:validate()returnsinvalid MetricsMode(config.go:158), andInit'sswitchhas adefaulterror case (observability.go:150).There is no
applyDefaultsfunction and nodefaults.gofile — the old doc invented both.Why it's a papercut
DefaultConfig()reads as "supplyServiceNameandMetricsModeand you're good." But:fails at runtime for what looks like a reasonable default path. Prometheus is the obvious default mode.
Decision needed
Option A — make code match the (old) doc intent: have
DefaultConfig()setMetricsMode = MetricsModePrometheus(and/or haveInitdefault a zero value). Friendlier; one fewer required field.Option B — keep
MetricsModerequired: current behavior; explicit over implicit. Docs already corrected to state this.If A: implement in
DefaultConfig()(no mutation inInit), add a test, re-documentMetricsModeas defaulted. If B: no code change — close this issue; the #189 doc fix already reflects it.Refs: #189 (doc correction).