hyperledger-firefly / hyperledger-firefly/common

Move the metrics-enabled toggle into pkg/metric

Open
#245 0 comments 1 reaction 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.