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.

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

[cassandra] Blacklisting/Whitelisting Tags for Indexing #1904

Merged
merged 23 commits into from
Nov 9, 2019

Conversation

joe-elliott
Copy link
Member

@joe-elliott joe-elliott commented Nov 7, 2019

Which problem is this PR solving?

Short description of the changes

  • Added two configuration options for the cassandra storage backend: tag-index-blacklist, tag-index-whitelist
  • Added appropriate TagFilters, Tests

@joe-elliott
Copy link
Member Author

At scale the canary ingester is using roughly the same CPU and indexing significantly less tags

image

@joe-elliott
Copy link
Member Author

Manually confirmed in our environment that blacklisted tags are not being indexed and other tags are. Removing draft.

@codecov
Copy link

codecov bot commented Nov 7, 2019

Codecov Report

Merging #1904 into master will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1904      +/-   ##
==========================================
+ Coverage   98.42%   98.45%   +0.03%     
==========================================
  Files         197      198       +1     
  Lines        9648     9706      +58     
==========================================
+ Hits         9496     9556      +60     
+ Misses        116      114       -2     
  Partials       36       36
Impacted Files Coverage Δ
plugin/storage/cassandra/factory.go 100% <100%> (ø) ⬆️
...sandra/spanstore/dbmodel/tag_filter_exact_match.go 100% <100%> (ø)
plugin/storage/cassandra/options.go 100% <100%> (ø) ⬆️
...lugin/sampling/strategystore/adaptive/processor.go 100% <0%> (+0.79%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2092718...a0bfe43. Read the comment docs.

Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

looks great, only a few small comments

plugin/storage/cassandra/options.go Outdated Show resolved Hide resolved
plugin/storage/cassandra/options.go Outdated Show resolved Hide resolved
plugin/storage/cassandra/options.go Outdated Show resolved Hide resolved
plugin/storage/cassandra/factory_test.go Outdated Show resolved Hide resolved
plugin/storage/cassandra/options.go Outdated Show resolved Hide resolved
plugin/storage/cassandra/options_test.go Outdated Show resolved Hide resolved
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

you should not have jaeger-ui submodule change in this PR. Please make sure the submodule points to the same revision as master

@yurishkuro
Copy link
Member

Please make sure the description says Resolves #1901 rather than just the issue number, to enable auto-close.

Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Signed-off-by: Joe Elliott <[email protected]>
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

Thanks!

@yurishkuro yurishkuro merged commit c8f52a9 into jaegertracing:master Nov 9, 2019
@yurishkuro yurishkuro changed the title Cassandra Tag Index Filters [cassandra] Blacklisting/Whitelisting Tags for Indexing Nov 9, 2019
@yurishkuro yurishkuro added this to the Release 1.16 milestone Nov 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow configuration of Cassandra tag indexing
2 participants