MWI: Add teleport_bot_instances metric#59774
Merged
Conversation
boxofrad
added
observability
boxofrad
temporarily deployed
to
docs-amplify
GitHub Actions
Inactive
github-actions
bot
requested review from
capnspacehook,
fspmarshall,
mmcallister,
ptgott,
r0mant and
zmb3
Contributor
|
Amplify deployment status
|
boxofrad
removed request for
capnspacehook,
fspmarshall,
mmcallister,
ptgott,
r0mant and
zmb3
boxofrad
force-pushed
the
boxofrad/bot-version-report
branch
from
e19e015 to
578873d
Compare
boxofrad
force-pushed
the
boxofrad/bot-version-metrics
branch
from
ec832ae to
e8377fc
Compare
boxofrad
temporarily deployed
to
docs-amplify
GitHub Actions
Inactive
boxofrad
temporarily deployed
to
docs-amplify
GitHub Actions
Inactive
hugoShaka
approved these changes
Oct 1, 2025
Comment on lines
+337
to
+342
| for version, count := range byVersion { | ||
| gauge.With(prometheus.Labels{ | ||
| teleport.TagVersion: version, | ||
| teleport.TagAutomaticUpdates: "true", | ||
| }).Set(float64(count)) | ||
| } |
Contributor
There was a problem hiding this comment.
This looks like an infinitely growing number of timeseries, never reset during the process lifetime. I'm not sure how much of an issue this is but this smells. You might want to leave a comment saying that no other labels should be added to this metric without further safeties to mitigate label cardinality.
For reference I wrote but never had time to merge a metrics RFD: https://github.com/gravitational/teleport/blob/92a08edc66a192a161a90ca5cf10162085d63d9c/rfd/0197-prometheus-metrics.md
Contributor
There was a problem hiding this comment.
This sounds like a problem we should actually address - since if you upgraded all your tbots from one version to another, they'd all still show up under the old version 😓 I think I ran into this in the past year or two - let me see if I can find how I resolved it.
Contributor
There was a problem hiding this comment.
So I've handled this two different ways previously:
- Just reset the GaugeVec before you write to it. This obviously leaves a very short period where it's state is "weird" but often if you've precomputed the values before you start writing into it - this period is very very short.
- Just directly implemented the Collector interface myself. I actually feel like I tend to prefer this to (1) since it's less hacky but you also benefit less from the guard rails the SDK provides.
Contributor
Author
There was a problem hiding this comment.
Thanks for linking to the RFD! That's a really useful resource.
On the resetting thing, I'm already calling gauge.Reset on L316, which I think is what @strideynet suggested above. Is that enough to drop the stale series?
strideynet
approved these changes
Oct 2, 2025
boxofrad
force-pushed
the
boxofrad/bot-version-metrics
branch
from
e8377fc to
d413d02
Compare
changelog: MWI: Add `teleport_bot_instances` metric
boxofrad
force-pushed
the
boxofrad/bot-version-metrics
branch
from
d413d02 to
4b8cc27
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
boxofrad
added a commit
that referenced
this pull request
Oct 13, 2025
boxofrad
added a commit
that referenced
this pull request
Oct 14, 2025
boxofrad
added a commit
that referenced
this pull request
Oct 15, 2025
boxofrad
added a commit
that referenced
this pull request
Oct 15, 2025
boxofrad
added a commit
that referenced
this pull request
Oct 15, 2025
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 15, 2025
* MWI: Generate `AutoUpdateBotInstanceReport` resource (#59738) * MWI: Add `tctl` get and delete mappings for `AutoUpdateBotInstanceReport` (#60017) * MWI: Add `teleport_bot_instances` metric (#59774) * MWI: Log on `AutoUpdateBotInstanceReport` generation failure (#60191) * Fix passing lock by value * Allow `machineid.AutoUpdateVersionReporter` to shut down correctly (#60219)
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 15, 2025
* MWI: Generate `AutoUpdateBotInstanceReport` resource (#59738) * MWI: Add `tctl` get and delete mappings for `AutoUpdateBotInstanceReport` (#60017) * MWI: Add `teleport_bot_instances` metric (#59774) * MWI: Log on `AutoUpdateBotInstanceReport` generation failure (#60191) * Allow `machineid.AutoUpdateVersionReporter` to shut down correctly (#60219)
rhammonds-teleport
pushed a commit
that referenced
this pull request
Nov 6, 2025
mmcallister
pushed a commit
that referenced
this pull request
Nov 19, 2025
mmcallister
pushed a commit
that referenced
this pull request
Nov 20, 2025
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
3 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.
Exposes the number of bot instances by version, so that Teleport Cloud and other operators can take it into account when performing cluster upgrades.
changelog: MWI: Add
teleport_bot_instancesmetric to track the number of bot instances across the cluster, by version