feat(operator): deprecate operatorconfig.collection.filter.matchOneOf #1688

bwplotka · 2025-06-27T10:43:01Z

After time we realized that a bug caused the operator config export filtering logic to be not used from the GMP 0.14 release onwards. We decided to keep this field NOT applicable (changing this field being noop) going forward, so this PR makes the deprecation/removal of this functionality clear with the operation and export code removed.

This feature since the beginning was extremely hard to use and confusing to users, users constantly assumed list of matchers are "AND-ed" while they are "OR-ed" or they are were filtering partial histograms (demo).
This feature is not stopping collectors (Prometheus) to scrape the filtered out series, it only NOT send them forward. This limits the use of them for collector efficiency scenarios.
We have evidences of users already starting to "misconfigure" their collection with this field, but they were "lucky" those filters are accidently turned off. "Fixing" them will immediately break those users in a hidden way.
Finally, better alternatives exists https://cloud.google.com/stackdriver/docs/managed-prometheus/setup-managed#filter-metrics

This feature, for dynamic config based setting in export pkg is already deprecated: GoogleCloudPlatform/prometheus#237

Post-mortem

For security hardening, when introducing reloadable config pieces (compression, credentials and matchers) in 0.14 we updated export package to load things from the correct place, but the matchers are initialized and copied into seriesCache and never updated later:

prometheus-engine/pkg/export/export.go

Line 419 in 10c5314

    
           seriesCache:          newSeriesCache(logger, reg, opts.MetricTypePrefix, opts.Matchers),

This affects both collector and rule-eval AFAIK.

We didn't have a good tests to test this e2e too.

AIs:

Update public docs
Remove this functionality (this PR and future)
Ensure proper e2e test coverage of compression option (credentialFile is tested).
Add this deprecation to the 0.14 changelog

NOTE: We don't plan to add deprecation warning to 0.15.

bwplotka · 2025-06-27T11:25:51Z

Updated changelog as well https://github.com/GoogleCloudPlatform/prometheus-engine/releases/tag/v0.14.0#:~:text=EDIT(June%202025,breakage%20before%20release.

pintohutch · 2025-07-01T19:43:03Z

prometheus-engine/pkg/export/export.go

Lines 1097 to 1107 in 77f0a33

    
           func (m *Matchers) Matches(lset labels.Labels) bool { 
        
           	if len(*m) == 0 { 
        
           		return true 
        
           	} 
        
           	for _, sel := range *m { 
        
           		if sel.Matches(lset) { 
        
           			return true 
        
           		} 
        
           	} 
        
           	return false 
        
           }

is a logical OR, no?

lyanco · 2025-07-07T14:19:11Z

I'm not sure anymore whether it was an OR or an AND... but also I very much remember people shooting themselves in the foot with this back in 2022 because it wasn't implemented the way people thought it would be implemented, to the point where we had to change the examples in the docs to be negative matchers on metrics that don't exist because people kept copy-pasting the older examples verbatim and accidentally filtering out all their metrics.

bwplotka · 2025-07-11T12:05:07Z

Yup, double checked and indeed it's OR-ed, which only proves user can get misleading. Another (production cx) example is with broken histograms (demo).

Anyway, PR still stands I think we should proceed PTAL @pintohutch @dashpole

@lyanco you are ok with this approach (deprecating and removing?)

bwplotka · 2025-07-11T14:46:52Z

While we wished we could reenabled this under some different, bit easier to use config API surface, the decision with @lyanco is to remove it for as other alternatives exists and focus on more important tasks.

cmd/rule-evaluator/main.go

dashpole

approving go code changes. I don't have context on whether this is a good idea or not :)

bwplotka · 2025-07-13T19:47:08Z

NOTE: Tests are failing on some struct unmarshalling. I fixed those errors on #1711 and I expect #1711 to be merged first and this conflict resolved and merged.

pintohutch · 2025-07-15T21:38:02Z

Given the footgun-ness of the feature as Lee described, I agree we should proceed with the PRs and be explicit in our kubebuilder tags to mark the CR field as deprecated.

manifests/setup.yaml

bwplotka · 2025-08-28T12:43:26Z

Should be rdy for review PTAL!

bernot-dev · 2025-08-28T15:27:45Z

Looks like tests are still failing.

bwplotka · 2025-08-28T15:28:32Z

Looks like tests are still failing.

Yea, I mentioned this on our standup. It's odd.. couldn't figure out why without local debugging session to do... will check tomorrow ):

bwplotka · 2025-08-29T03:15:57Z

OK, I found the bug -- it's due to me extending the tests, not related to this change.

Fixed in another PR (#1751) and chained this PR on it.

bwplotka · 2025-08-29T03:54:07Z

Don't merge before #1751 is merged pls (:

After time we realized that a bug caused the operator config export filtering logic to be not used from the GMP 0.14 release onwards. We decided to keep this field NOT applicable (changing this field being noop) going forward, so this PR makes the deprecation/removal of this functionality clear with the operation and export code removed. * This feature since the beginning was extremely hard to use and confusing to users, users constantly assumed list of matchers are "AND-ed" while they are "OR-ed" or they are were filtering partial histograms ([demo](a74a72f)). * This feature is not stopping collectors (Prometheus) to scrape the filtered out series, it only NOT send them forward. This limits the use of them for collector efficiency scenarios. * We have evidences of users already starting to "misconfigure" their collection with this field, but they were "lucky" those filters are accidentally turned off. "Fixing" them will immediately break those users in a hidden way. * Finally, better alternatives exists https://cloud.google.com/stackdriver/docs/managed-prometheus/setup-managed#filter-metrics This feature, for dynamic config based setting in export pkg is already deprecated: GoogleCloudPlatform/prometheus#237 For security hardening, when introducing reloadable config pieces (compression, credentials and matchers) in 0.14 we updated export package to load things from the correct place, but the matchers are initialized and copied into seriesCache and never updated later: https://github.com/GoogleCloudPlatform/prometheus-engine/blob/10c5314b02cc99bd0ee2e71f2588b7aac870c329/pkg/export/export.go#L419 This affects both collector and rule-eval AFAIK. We didn't have a good tests to test this e2e too. AIs: * [X] Update public docs * [x] Remove this functionality (this PR and future) * [x] Ensure proper e2e test coverage of compression option (credentialFile is tested). * [X] Add this deprecation to the 0.14 changelog NOTE: We don't plan to add deprecation warning to 0.15. Signed-off-by: bwplotka <bwplotka@google.com>

bwplotka · 2025-08-29T18:53:32Z

Given new revelations, let's reconsider this (:

bwplotka requested review from TheSpiritXIII, bernot-dev and pintohutch June 27, 2025 10:43

bwplotka changed the title ~~feat(operator): deprecate the operatorconfig.collection.filter.matchOneOf functionality~~ feat(operator): deprecate operatorconfig.collection.filter.matchOneOf Jun 27, 2025

bwplotka force-pushed the filtering branch 2 times, most recently from 26d83ae to 4941d55 Compare June 27, 2025 11:41

bwplotka requested a review from dashpole July 11, 2025 12:05

bwplotka force-pushed the filtering branch from 4941d55 to db8e9f3 Compare July 11, 2025 12:05

dashpole reviewed Jul 11, 2025

View reviewed changes

cmd/rule-evaluator/main.go Outdated Show resolved Hide resolved

dashpole approved these changes Jul 11, 2025

View reviewed changes

bwplotka force-pushed the filtering branch 2 times, most recently from cda7708 to d0b8468 Compare August 28, 2025 10:21

bwplotka commented Aug 28, 2025

View reviewed changes

manifests/setup.yaml Show resolved Hide resolved

bwplotka force-pushed the filtering branch 2 times, most recently from af99d2e to df6370a Compare August 28, 2025 12:42

bwplotka force-pushed the filtering branch 2 times, most recently from 792de57 to a685b1d Compare August 28, 2025 14:53

bwplotka force-pushed the filtering branch 3 times, most recently from c2a6fae to f9557b5 Compare August 29, 2025 02:58

bwplotka changed the title ~~feat(operator): deprecate operatorconfig.collection.filter.matchOneOf~~ feat(operator): deprecate operatorconfig(..)matchOneO; fix collection compression propagation Aug 29, 2025

bwplotka changed the title ~~feat(operator): deprecate operatorconfig(..)matchOneO; fix collection compression propagation~~ feat(operator): deprecate operatorconfig.collection.filter.matchOneOf Aug 29, 2025

bwplotka mentioned this pull request Aug 29, 2025

fix: operator propagate correct compression for collection #1751

Merged

bwplotka force-pushed the filtering branch from f9557b5 to 72173fa Compare August 29, 2025 03:38

bwplotka changed the base branch from main to compression-fix August 29, 2025 03:38

bwplotka force-pushed the filtering branch from 72173fa to 8816101 Compare August 29, 2025 03:39

bwplotka requested a review from dashpole August 29, 2025 03:54

bwplotka force-pushed the compression-fix branch 2 times, most recently from 2f4f7c7 to 4bd847d Compare August 29, 2025 04:19

bwplotka changed the base branch from compression-fix to main August 29, 2025 15:04

bwplotka force-pushed the filtering branch from 8816101 to 7510f93 Compare August 29, 2025 15:37

bwplotka closed this Aug 29, 2025

bwplotka mentioned this pull request Sep 1, 2025

The PodMonitoring metricRelabeling do not work correctly #1753

Closed

feat(operator): deprecate operatorconfig.collection.filter.matchOneOf #1688

feat(operator): deprecate operatorconfig.collection.filter.matchOneOf #1688

Uh oh!

Conversation

bwplotka commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Post-mortem

Uh oh!

bwplotka commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pintohutch commented Jul 1, 2025

Uh oh!

lyanco commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bwplotka commented Jul 11, 2025

Uh oh!

bwplotka commented Jul 11, 2025

Uh oh!

Uh oh!

dashpole left a comment

Choose a reason for hiding this comment

Uh oh!

bwplotka commented Jul 13, 2025

Uh oh!

pintohutch commented Jul 15, 2025

Uh oh!

Uh oh!

bwplotka commented Aug 28, 2025

Uh oh!

bernot-dev commented Aug 28, 2025

Uh oh!

bwplotka commented Aug 28, 2025

Uh oh!

bwplotka commented Aug 29, 2025

Uh oh!

bwplotka commented Aug 29, 2025

Uh oh!

bwplotka commented Aug 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bwplotka commented Jun 27, 2025 •

edited

Loading

bwplotka commented Jun 27, 2025 •

edited

Loading

lyanco commented Jul 7, 2025 •

edited

Loading