-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Stack Monitoring] Add stack_monitoring suffix to metrics-* index pattern #137904
[Stack Monitoring] Add stack_monitoring suffix to metrics-* index pattern #137904
Conversation
Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI) |
Looks good. Just a note that until #123508 is complete, there are still a few places that won't be updated yet. |
@elasticmachine merge upstream |
buildkite test this |
@elasticmachine merge upstream |
Looks like just a docker blip on the last run so re-updating |
Should we block this PR until mappings alignment is complete (only logstash left elastic/integrations#3993) ? We can then update the package's data streams to write to the new |
When updating a data stream's dataset, not only the index name changes but also the |
…-ref HEAD~1..HEAD --fix'
@elasticmachine merge upstream |
x-pack/plugins/monitoring/server/lib/cluster/get_index_patterns.ts
Outdated
Show resolved
Hide resolved
@@ -123,7 +123,9 @@ export function getDsIndexPattern({ | |||
config, | |||
ccs, | |||
}: CommonIndexPatternArgs & { type?: string }): string { | |||
const datasetsPattern = `${moduleType ?? '*'}.${dataset ?? '*'}`; | |||
const datasetsPattern = `${moduleType ?? '*'}.${ | |||
type === DS_INDEX_PATTERN_METRICS ? 'stack_monitoring.' : '' |
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.
Added this condition when resolving conflicts and looks like this function is doing too much. We could split it in getMetricsIndexPatterns
and getLogsIndexPatterns
. the split itself is reasonable and makes more sense with the dataset branching now
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.
thanks @klacabane. I'll try to improve this.
@elasticmachine merge upstream |
Pretty sure we're still holding this PR, so I added the blocked label to make sure I'm reviewing things in a good order :) |
When testing elastic/integrations#4018 I found that this branch produces a "ghost" standalone cluster when logstash connection errors are present. Could be that's out of scope for this PR, but it'd be a little weird to ship kibana with this behavior (assuming we haven't already). Update: elastic/integrations#4011 is open to fix that so we probably can skip it for this PR |
The ghost standalone will be fixed with #140102 and elastic/integrations#4147 |
Should we wait those fixes before merging this one? |
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: |
I'm inclined to merge this now since the corresponding integration PR was merged, and the standalone issue does not impact metricbeat/legacy collection mode. |
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.
Agreed. I've been using this to test the standalone changes and it seems great.
Summary
This PR closes #135382 by updating
metrics-*
index pattern - which are generated by the integration packages -, addingstack_monitoring
to it, so that the UI can reflect this update on the packagesIt prepares the SM server code for the index pattern update that will happen on the SM integration packages
For maintainers
There is actually a limit of 255 characters for index names for index name, but I don't expect that adding this suffix will cause problems in the future. Considering the following example:
.ds-metrics-elasticsearch.index_recovery-default-2022.08.02-000001
= 66 charactersThis PR can only be merged once the integration packages indices have been updated