Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support global denylist and per-tenant allowlist of tags for search data #960

Merged
merged 13 commits into from
Sep 23, 2021
Merged

Support global denylist and per-tenant allowlist of tags for search data #960

merged 13 commits into from
Sep 23, 2021

Conversation

annanay25
Copy link
Contributor

@annanay25 annanay25 commented Sep 15, 2021 •
edited
Loading

Signed-off-by: Annanay annanayagarwal@gmail.com

What this PR does:

  • Support global denylist of tags to be dropped from search data. The denylist will be applied at the distributors and only filtered tags will be added to the flatbuffers.
  • Support per-tenant allowlist of tags that will override the global denylist.

Which issue(s) this PR fixes:
Fixes part of #932

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@annanay25
[search] Add global deny list of tags
1a0f402 
Signed-off-by: Annanay <annanayagarwal@gmail.com>
@annanay25 annanay25 changed the title Add global denylist of tags to be added to search data Support global denylist of tags to be dropped from search data Sep 15, 2021
@annanay25
Move CHANGELOG entries around, add docs
7405e91 
Signed-off-by: Annanay <annanayagarwal@gmail.com>
@annanay25
Merge branch 'main' into search-tag-denylist
dbbb8d8 
Signed-off-by: Annanay <annanayagarwal@gmail.com>
@annanay25
Fix entry
a349d7c 
Signed-off-by: Annanay <annanayagarwal@gmail.com>
@annanay25 annanay25 mentioned this pull request Sep 15, 2021
Closed
12 tasks
mapno
mapno approved these changes Sep 16, 2021
View reviewed changes
Copy link
Member

@mapno mapno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

mapno
mapno reviewed Sep 20, 2021
View reviewed changes
modules/distributor/distributor.go Outdated Show resolved Hide resolved
modules/distributor/tag_manager.go Outdated Show resolved Hide resolved
modules/distributor/tag_manager.go Outdated Show resolved Hide resolved
@annanay25
Rework around adding a custom type and yaml/json marshaller
7409d4b 
Signed-off-by: Annanay <annanayagarwal@gmail.com>
@annanay25
Merge branch 'main' into search-tag-denylist
a8f1d7d 
Signed-off-by: Annanay <annanayagarwal@gmail.com>
@annanay25 annanay25 changed the title Support global denylist of tags to be dropped from search data Support global denylist and per-tenant allowlist of tags for search data Sep 21, 2021
@annanay25
Checkpoint: Implemented marshal funcs and added test
c97995c 
Signed-off-by: Annanay <annanayagarwal@gmail.com>
@annanay25
Merge branch 'main' into search-tag-denylist
5a1b8bd 
Signed-off-by: Annanay <annanayagarwal@gmail.com>
@annanay25
Improve tests, handle edge cases
0266c39 
Signed-off-by: Annanay <annanayagarwal@gmail.com>
joe-elliott
joe-elliott approved these changes Sep 22, 2021
View reviewed changes
Copy link
Member

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a couple of small things to check, but overall looks great!

modules/overrides/overrides.go Show resolved Hide resolved
modules/overrides/list_to_map.go Outdated Show resolved Hide resolved
@annanay25
Address comments
b06385a 
Signed-off-by: Annanay <annanayagarwal@gmail.com>
@annanay25
Test denylist/allowlist in docker-compose tempo-search example
d3745b2 
Signed-off-by: Annanay <annanayagarwal@gmail.com>
@annanay25
Fix single-tenant overrides
a87ae20 
Signed-off-by: Annanay <annanayagarwal@gmail.com>
@annanay25
Copy link
Contributor Author

annanay25 commented Sep 23, 2021 •
edited
Loading

Thanks @joe-elliott, addressed both.

Feeling really good about the state of this. Tested with the docker-compose example as well. Will merge on green CI.

@annanay25
Merge branch 'main' into search-tag-denylist
6b2a3ec 
Signed-off-by: Annanay <annanayagarwal@gmail.com>
@annanay25 annanay25 merged commit 1a46c46 into grafana:main Sep 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mapno mapno mapno approved these changes

@joe-elliott joe-elliott joe-elliott approved these changes

@dgzlopes dgzlopes Awaiting requested review from dgzlopes

@kvrhdn kvrhdn Awaiting requested review from kvrhdn kvrhdn is a code owner

@mdisibio mdisibio Awaiting requested review from mdisibio mdisibio is a code owner

@achatterjee-grafana achatterjee-grafana Awaiting requested review from achatterjee-grafana

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@annanay25 @joe-elliott @mapno