-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
#2048 - Support Elasticsearch ILM for managing jaeger indices #2796
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2796 +/- ##
==========================================
+ Coverage 95.91% 95.94% +0.03%
==========================================
Files 218 221 +3
Lines 9620 9693 +73
==========================================
+ Hits 9227 9300 +73
Misses 324 324
Partials 69 69
Continue to review full report at Codecov.
|
codecov is acting out again, but the report looks ok to merge: https://codecov.io/github/jaegertracing/jaeger/commit/49e6f871181214f3a29142c074660089692d8f9d |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, but I have some suggestions on refactoring for clarity/consistency
…dices Signed-off-by: santosh <bsantosh@thoughtworks.com>
Signed-off-by: santosh <bsantosh@thoughtworks.com>
Pull request has been modified.
Signed-off-by: santosh <bsantosh@thoughtworks.com>
Rebased with master @yurishkuro - Kindly approve |
@@ -232,6 +232,12 @@ func addFlags(flagSet *flag.FlagSet, nsConfig *namespaceConfig) { | |||
"Use read and write aliases for indices. Use this option with Elasticsearch rollover "+ | |||
"API. It requires an external component to create aliases before startup and then performing its management. "+ | |||
"Note that "+nsConfig.namespace+suffixMaxSpanAge+" will influence trace search window start times.") | |||
flagSet.Bool( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's fine @bhiravabhatla.
cmd/opentelemetry/app/exporter/elasticsearchexporter/exporter.go
Outdated
Show resolved
Hide resolved
cmd/opentelemetry/app/exporter/elasticsearchexporter/integration_test.go
Outdated
Show resolved
Hide resolved
Signed-off-by: santosh <bsantosh@thoughtworks.com>
@yurishkuro Can we merge this PR to master - so that I can take rebase and create a new branch to work on for followup PRs. |
done |
Signed-off-by: santosh bsantosh@thoughtworks.com
Which problem is this PR solving?
Short description of the changes