hyperledger-firefly / hyperledger-firefly/common
Move the metrics-enabled toggle into pkg/metric
- Dominant language
- Go
- Stars
- 5
- Forks
- 15
- Avg merge
- 3d 6h
- Merged PRs (30d)
- 4
Description
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`.
Contributor guide
Research direction
Start by exploring the pkg/metric package, especially metric.Options, NewPrometheusMetricsRegistryWithOptions, MetricsRegistry, and NewMetricsManagerForSubsystem. Compare the current enablement wrappers in transaction-manager's internal/metrics/metrics.go and event_metrics.go. Done means disabled registries and managers consistently no-op while the default remains enabled, including handles passed through MetricsRegistry().
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, prometheus
- Domain
- backend, observability-sre
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 58/100