Ingester: Add matchers to LabelNames() ingester RPC #6209
Conversation
harry671003
changed the title
harry671003
force-pushed
the
add_match_to_labels
branch
from
2b01472
to
a9a2483
Compare
harry671003
changed the title
harry671003
force-pushed
the
add_match_to_labels
branch
from
a9a2483
to
5262d1e
Compare
harry671003
changed the title
| @@ -109,6 +109,10 @@ querier: | |||
| # CLI flag: -querier.ingester-metadata-streaming | |||
| [ingester_metadata_streaming: <boolean> | default = true] | |||
|
|
|||
| # Use LabelNames ingester RPCs with match params. | |||
| # CLI flag: -querier.ingester-label-names-with-matchers | |||
| [ingester_label_names_with_matchers: <boolean> | default = false] | |||
There was a problem hiding this comment.
Do we need this config?
If the matcher is in the request, its not cause we always wanna filter out?
There was a problem hiding this comment.
This feature flag is for safe guarding during a deployment.
In this PR, queriers will start calling ingester LabelNames with Matchers. But if ingesters haven't restarted yet, then ingesters will ignore that and return label names without filtering out.
We can remove this flag in 2 releases and make this behavior the default.
There was a problem hiding this comment.
I suggest we add TODO or create an issue about removing/deprecating this flag now. Otherwise we will forget
There was a problem hiding this comment.
I was thinking that if the ingesters ignored the matchers in the request from the querier since the ingesters had not restarted yet, wouldn't that be the same behavior today since the ingesters already do not handle the matchers?
There was a problem hiding this comment.
Today's behavior for label names calls with matchers is like this:
- Querier will retrieve all the series using
MetricsForLabelMatcherscall. It'll then get the unique label names from the retrieved series.
The new behaviour is for label names calls with matchers:
- Querier will directly call
LabelNamescall and pass the matchers. - If ingesters ignore the matchers, this will break the API.
I think having the feature flag is unavoidable.
Created #6222 for tracking the deprecation of the flag.
There was a problem hiding this comment.
I see, thank you for the clarification @harry671003.
Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
harry671003
force-pushed
the
add_match_to_labels
branch
from
5262d1e
to
a9a6dd1
Compare
What this PR does:
Prometheus has supported matchers in LabelNames() API also. This change will pass the matchers to LabelNames() call on ingesters.
The main motivation is to improve the efficiency of /api/v1/labels and avoid using the MetricsWithLabelMatchers() call when it's not necessary.
Which issue(s) this PR fixes:
Fixes #
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]