Optionally separate metrics by group #2702
Closed
Conversation
This file contains 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
LeviHarrison
commented
Aug 11, 2022
Comment on lines
+478
to
+496
| if err := util.DeleteMatchingLabels(d.receivedSamples, filter); err != nil { | ||
| level.Warn(d.log).Log("msg", "failed to remove cortex_distributor_received_samples_total metric for user", "user", userID, "err", err) | ||
| } | ||
| if err := util.DeleteMatchingLabels(d.receivedExemplars, filter); err != nil { | ||
| level.Warn(d.log).Log("msg", "failed to remove cortex_distributor_received_exemplars_total metric for user", "user", userID, "err", err) | ||
| } | ||
| if err := util.DeleteMatchingLabels(d.receivedMetadata, filter); err != nil { | ||
| level.Warn(d.log).Log("msg", "failed to remove cortex_distributor_received_metadata_total metric for user", "user", userID, "err", err) | ||
| } | ||
| if err := util.DeleteMatchingLabels(d.incomingSamples, filter); err != nil { | ||
| level.Warn(d.log).Log("msg", "failed to remove cortex_distributor_samples_in_total metric for user", "user", userID, "err", err) | ||
| } | ||
| if err := util.DeleteMatchingLabels(d.incomingExemplars, filter); err != nil { | ||
| level.Warn(d.log).Log("msg", "failed to remove cortex_distributor_exemplars_in_total metric for user", "user", userID, "err", err) | ||
| } | ||
| if err := util.DeleteMatchingLabels(d.incomingMetadata, filter); err != nil { | ||
| level.Warn(d.log).Log("msg", "failed to remove cortex_distributor_metadata_in_total metric for user", "user", userID, "err", err) | ||
| } | ||
| if err := util.DeleteMatchingLabels(d.dedupedSamples, filter); err != nil { |
There was a problem hiding this comment.
This is a little bit ugly, but #2660 is on hold right now.
| @@ -41,13 +41,14 @@ type seriesStripe struct { | |||
|
|
|||
| mu sync.RWMutex | |||
| refs map[uint64][]seriesEntry | |||
| active int // Number of active entries in this stripe. Only decreased during purge or clear. | |||
| activeMatching []int // Number of active entries in this stripe matching each matcher of the configured Matchers. | |||
| active map[string]int // Number of active entries in this stripe per group. Only decreased during purge or clear. | |||
There was a problem hiding this comment.
I assume there's some performance impact of moving from the plain integer to a map. Because the vast majority of tenants will probably not be using this feature and the active series tracker is provisioned per-tenant, we could keep the old integer variable and only use the map if the tenant is using the separating metrics feature. Not sure if that's worth it thought.
Signed-off-by: Levi Harrison <git@leviharrison.dev>
LeviHarrison
force-pushed
the
separate-metrics
branch
from
41f0060
to
1d323da
Compare
|
The CHANGELOG has just been cut to prepare for the next Mimir release. Please rebase |
3 tasks
|
Closing, as #3439 implementing the same idea was merged, and this PR is abandoned. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What this PR does
The PR implements a feature which I'm calling "separating metrics," which breaks out some per-tenant metrics by the label
group, which can be configured to be the value of any label on incoming series, by defaultteam. Sort of like how the HA tracker works with theclusterlabel. The feature can be enabled per-tenant.A few open questions:
grouplabel to are listed below. The all fall under the general categories of usage/billing and errors. Any other ones we want or are there some of these that we don't want?cortex_distributor_received_samples_totalcortex_distributor_received_exemplars_totalcortex_distributor_received_metadata_totalcortex_distributor_samples_in_totalcortex_distributor_exemplars_in_total,cortex_distributor_metadata_in_totalcortex_discarded_samples_totalcortex_discarded_exemplars_totalcortex_discarded_metadata_totalcortex_ingester_ingested_samples_totalcortex_ingester_ingested_exemplars_totalcortex_ingester_ingested_samples_failures_totalcortex_ingester_active_series--enable-separating-metricsand--separate-metrics-labelcan be used by either the distributor or the ingester, I didn't prefix them with component (ex.--distributor.enable-separating-metrics). Is that the proper way to do things?Note: When the group label is empty, it gets dropped when scraped by Prometheus. So we don't have to worry about whether or not the label should be set even if the feature isn't enabled.
Which issue(s) this PR fixes or relates to
Fixes #2420
Checklist
Waiting to update tests and docs until naming/metrics/approach are confirmed.
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]