Move the metrics-enabled toggle into pkg/metric
Problem
pkg/metric's MetricsRegistry/MetricsManager interfaces have no concept of "enabled". Every New*Metric, Set*, Inc*, and Observe* method unconditionally does the real prometheus work.
Because of that, every consuming service has to hand-roll its own enablement gate around each call. transaction-manager is a good example: internal/metrics/metrics.go computes a local metricsEnabled bool from config (monitoring.enabled / deprecated metrics.enabled), then wraps roughly 20 separate methods (across metrics.go and event_metrics.go) in if mm.metricsEnabled { ... }, one at a time.
This duplication is easy to get wrong or incomplete. We recently found one gap in transaction-manager: Manager.MetricsRegistry() hands the raw registry to downstream connector code (for registering custom metrics) without exposing whether metrics are actually enabled, so callers had no way to check before registering. That particular gap is fixed locally, but the underlying pattern (each service reimplementing its own enabled/disabled wrapper) is duplicated per-repo and will likely resurface elsewhere in the firefly microservice fleet.
Proposal
Move the enabled/disabled concept into pkg/metric itself:
- Add an
Enabled bool field to metric.Options (default true), passed through NewPrometheusMetricsRegistryWithOptions.
- Expose
IsEnabled() bool on MetricsRegistry.
- Have the
MetricsManager returned by NewMetricsManagerForSubsystem self-gate its New*Metric/Set*/Inc*/Observe* calls on that flag, becoming a no-op when disabled.
With that in place, consuming services no longer need a local enabled bool or per-method gating: they set Enabled once at registry construction time based on their own config, and every metrics call and every handle obtained from the registry (including MetricsRegistry()-style pass-throughs to downstream code) is consistent by construction.
Reference
Context from hyperledger-firefly/transaction-manager: internal/metrics/metrics.go, internal/metrics/event_metrics.go.
Move the metrics-enabled toggle into pkg/metric
Problem
pkg/metric'sMetricsRegistry/MetricsManagerinterfaces have no concept of "enabled". EveryNew*Metric,Set*,Inc*, andObserve*method unconditionally does the real prometheus work.Because of that, every consuming service has to hand-roll its own enablement gate around each call. transaction-manager is a good example:
internal/metrics/metrics.gocomputes a localmetricsEnabled boolfrom config (monitoring.enabled/ deprecatedmetrics.enabled), then wraps roughly 20 separate methods (acrossmetrics.goandevent_metrics.go) inif mm.metricsEnabled { ... }, one at a time.This duplication is easy to get wrong or incomplete. We recently found one gap in transaction-manager:
Manager.MetricsRegistry()hands the raw registry to downstream connector code (for registering custom metrics) without exposing whether metrics are actually enabled, so callers had no way to check before registering. That particular gap is fixed locally, but the underlying pattern (each service reimplementing its own enabled/disabled wrapper) is duplicated per-repo and will likely resurface elsewhere in the firefly microservice fleet.Proposal
Move the enabled/disabled concept into
pkg/metricitself:Enabled boolfield tometric.Options(defaulttrue), passed throughNewPrometheusMetricsRegistryWithOptions.IsEnabled() boolonMetricsRegistry.MetricsManagerreturned byNewMetricsManagerForSubsystemself-gate itsNew*Metric/Set*/Inc*/Observe*calls on that flag, becoming a no-op when disabled.With that in place, consuming services no longer need a local enabled bool or per-method gating: they set
Enabledonce at registry construction time based on their own config, and every metrics call and every handle obtained from the registry (includingMetricsRegistry()-style pass-throughs to downstream code) is consistent by construction.Reference
Context from hyperledger-firefly/transaction-manager:
internal/metrics/metrics.go,internal/metrics/event_metrics.go.