Introduce metrics.Registry to pass down registries#61239
Merged
Conversation
hugoShaka
force-pushed
the
hugo/propagate-metric-namespace-and-subsystem
branch
from
ce1e0ad to
d3b1564
Compare
hugoShaka
force-pushed
the
hugo/propagate-metric-namespace-and-subsystem
branch
from
d3b1564 to
17a82fd
Compare
hugoShaka
force-pushed
the
hugo/propagate-metric-namespace-and-subsystem
branch
from
17a82fd to
10a757e
Compare
hugoShaka
commented
Nov 11, 2025
Comment on lines
+33
to
+37
| type Registry struct { | ||
| prometheus.Registerer | ||
|
|
||
| namespace string | ||
| subsystem string |
Contributor
Author
There was a problem hiding this comment.
We can easily add a debug prometheus.Registerer later, so the scale team can expose tctl debug-only super high cardinality metrics.
hugoShaka
added
the
no-changelog
hugoShaka
changed the title
rosstimothy
reviewed
Nov 11, 2025
Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
rosstimothy
approved these changes
Nov 11, 2025
Contributor
Author
|
Must be merged after https://github.com/gravitational/teleport.e/pull/7557 to avoid breaking e builds |
espadolini
approved these changes
Nov 12, 2025
public-teleport-github-review-bot
bot
removed request for
gzdunek,
r0mant and
strideynet
strideynet
approved these changes
Nov 12, 2025
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
codingllama
reviewed
Nov 14, 2025
Contributor
codingllama
left a comment
codingllama
left a comment
There was a problem hiding this comment.
Drive-by from the prometheus RFD
| newReg := &Registry{ | ||
| Registerer: r.Registerer, | ||
| namespace: r.namespace, | ||
| subsystem: r.subsystem + "_" + subsystem, |
Contributor
There was a problem hiding this comment.
Should this handle an empty r.subsystem?
hugoShaka
added a commit
that referenced
this pull request
Nov 21, 2025
* 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>
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When working on plugin metrics I realized there are several metrics challenges in teleport:
teleport_namespace (tbot, teleport plugins, ...)This PR introduces a new
metrics.Registrytype that can be passed like a logger and contains information about:Putting everything into the same structs avoids cumbersome multiple argument while being able to properly nest metrics.