metrics.Registry.Wrap() handle empty subsystems properly#61392
Merged
metrics.Registry.Wrap() handle empty subsystems properly#61392
Conversation
hugoShaka
added
the
no-changelog
codingllama
approved these changes
Nov 14, 2025
Contributor
codingllama
left a comment
codingllama
left a comment
There was a problem hiding this comment.
LGTM.
Thanks for the quick fix!
kshi36
approved these changes
Nov 14, 2025
public-teleport-github-review-bot
bot
removed the request for review
from tigrato
tigrato
approved these changes
Nov 14, 2025
codingllama
approved these changes
Nov 14, 2025
Contributor
codingllama
left a comment
codingllama
left a comment
There was a problem hiding this comment.
appeasing our italian engineering team
😂
hugoShaka
added a commit
that referenced
this pull request
Nov 21, 2025
* handle empty subsystems properly * appeasing our italian engineering team
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
4 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.
Properly handles empty subsystems during wrapping.
Fixes: https://github.com/gravitational/teleport/pull/61239/files/a24bba394eb3aeddc2a83497a83f0bf4661772d5#r2528783100