Skip to content

Add support in process for additional metrics gatherers#60852

Merged
hugoShaka merged 2 commits intomasterfrom
hugo/metrics-additional-gatherers
Nov 8, 2025
Merged

Add support in process for additional metrics gatherers#60852
hugoShaka merged 2 commits intomasterfrom
hugo/metrics-additional-gatherers

Conversation

@hugoShaka
Copy link
Copy Markdown
Contributor

Before this change, we were gathering from 2 metrics gatherers:

  • the process registry
  • the global registry

There are cases where we must add and remove metrics (e.g. plugins). We could throw them into the global registry but:

  • this would pollute the global registry and cause duplicates/conflicts in tests
  • this would conflate all metrics from the same plugin kind. We support several instances of the same hosted plugin and we might want to keep distinct metrics.

This change makes the gatherers a list, and add a function so teleport.e can add its own gatherer. A teleport.e PR using this mechanism will follow.

@hugoShaka hugoShaka added the no-changelog Indicates that a PR does not require a changelog entry label Oct 30, 2025
@hugoShaka hugoShaka force-pushed the hugo/metrics-additional-gatherers branch from 66c3e94 to d053cf2 Compare October 30, 2025 22:31
@hugoShaka hugoShaka marked this pull request as ready for review October 30, 2025 23:20
@hugoShaka hugoShaka requested a review from rosstimothy October 30, 2025 23:24
Comment on lines +741 to +743
func (process *TeleportProcess) AddMetricsGatherer(gatherer prometheus.Gatherer) {
process.metricsGatherers = append(process.metricsGatherers, gatherer)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be mutex protected?

@hugoShaka hugoShaka force-pushed the hugo/metrics-additional-gatherers branch from d053cf2 to c1fbe9f Compare November 3, 2025 16:13
@hugoShaka hugoShaka marked this pull request as draft November 4, 2025 13:22
@hugoShaka hugoShaka force-pushed the hugo/metrics-additional-gatherers branch from c1fbe9f to a3f8379 Compare November 4, 2025 14:00
@hugoShaka hugoShaka marked this pull request as ready for review November 4, 2025 14:02
Before this change, we were gathering from 2 metrics gatherers:
- the process registry
- the global registry

There are cases where we must add and remove metrics (e.g. plugins).
We could throw them into the global registry but:
- this would pollute the global registry and cause duplicates/conflicts
  in tests
- this would conflate all metrics from the same plugin kind. We support
  several instances of the same hosted plugin and we might want to
  keep distinct metrics.

This change makes the gatherers a list, and add a function so teleport.e
can add its own gatherer. A teleport.e PR using this mechanism will
follow.
@hugoShaka hugoShaka force-pushed the hugo/metrics-additional-gatherers branch from de5e697 to 29c54b1 Compare November 4, 2025 16:00
@hugoShaka hugoShaka added this pull request to the merge queue Nov 8, 2025
Merged via the queue into master with commit 15219cf Nov 8, 2025
41 checks passed
@hugoShaka hugoShaka deleted the hugo/metrics-additional-gatherers branch November 8, 2025 01:43
hugoShaka added a commit that referenced this pull request Nov 21, 2025
* Add support in process for additional metrics gatherers

Before this change, we were gathering from 2 metrics gatherers:
- the process registry
- the global registry

There are cases where we must add and remove metrics (e.g. plugins).
We could throw them into the global registry but:
- this would pollute the global registry and cause duplicates/conflicts
  in tests
- this would conflate all metrics from the same plugin kind. We support
  several instances of the same hosted plugin and we might want to
  keep distinct metrics.

This change makes the gatherers a list, and add a function so teleport.e
can add its own gatherer. A teleport.e PR using this mechanism will
follow.

* Protect gatherer slice with a mutex
github-merge-queue bot pushed a commit that referenced this pull request Nov 24, 2025
* Add entra ID metrics (#60537)

* Add entra ID metrics

This commit adds metrics for entra ID sync. This is the OSS part, it
contains the msgraph client metrics.

As many different parts of Teleport are using the msgraph client and
might not have access to a metric registerer yet, the client gracefully
handles not being given a metric registry. In this case it won't
register its metrics, we don't want to continue polluting the global
metrics registry.

* lint

* add optional reconciler metrics (#60581)

* expose TeleportProcess metrics registry (#60654)

* test setting a non-nil registry in config

* expose teleport process metric registry

* remove metric config

* fixup! remove metric config

* Add support in process for additional metrics gatherers (#60852)

* Add support in process for additional metrics gatherers

Before this change, we were gathering from 2 metrics gatherers:
- the process registry
- the global registry

There are cases where we must add and remove metrics (e.g. plugins).
We could throw them into the global registry but:
- this would pollute the global registry and cause duplicates/conflicts
  in tests
- this would conflate all metrics from the same plugin kind. We support
  several instances of the same hosted plugin and we might want to
  keep distinct metrics.

This change makes the gatherers a list, and add a function so teleport.e
can add its own gatherer. A teleport.e PR using this mechanism will
follow.

* Protect gatherer slice with a mutex

* Fix the generic reconciler metric API (#60853)

When implementing reconciler metrics in #60581
I did not realize some GenericReconciler usage, including the one I
wanted to observe, were short-lived. The implementation had 2 blatant
issues:
- metrics were lost for each invocations
- creating a new reonciler would attempt to register the metric a second
  time and cause a conflict

This PR changes the reconciler metrics API so the caller is responsible
for creating and registering the metrics beforehand. This allows the
caller to create the metric struct once and pass them to successive
`NewGenericReconciler` calls.

* Introduce metrics.Registry to pass down registries (#61239)

* Introduce metrics.Registry and use it

* Update lib/metrics/registry.go

Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>

* BlackHole -> BlackHoleRegistry

* merge lib/metrics and lib/observability/metrics

* lint

* address noah's feedback

---------

Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>

* metrics.Registry.Wrap() handle empty subsystems properly (#61392)

* handle empty subsystems properly

* appeasing our italian engineering team

* Fix build after rebase

---------

Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Indicates that a PR does not require a changelog entry size/sm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants