-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Monitoring] Fixed internal monitoring check #79241
Conversation
Pinging @elastic/stack-monitoring (Team: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.
This isn't part of this PR, but why don't we have a range query on this: https://github.com/elastic/kibana/pull/79241/files#diff-e856f71a4bdab161dc03994cf4d8317fR21?
If the user has migrated from legacy to Metricbeat collection, they will still have their old legacy indices around for at least 7 days. Maybe we should only look back one day or something? Or maybe we look at the timestamp of the last legacy collected document and determine if it's enough time back, and we see a -mb-
one too?
.../plugins/monitoring/server/routes/api/v1/elasticsearch_settings/check/internal_monitoring.ts
Outdated
Show resolved
Hide resolved
@elasticmachine merge upstream |
@@ -69,9 +69,9 @@ export function monitoringClustersProvider($injector) { | |||
if (Legacy.shims.isCloud) { | |||
return Promise.resolve(); | |||
} | |||
|
|||
const css = window.location.hash.split('?')[0] === '#/home' ? 'css' : 'single'; |
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.
I'm not a big fan of hard-coding a URL when I think we can manage without it.
monitoringClusters
is only called from a few places, and we can just provide an additional parameter to the function call for ccs
and we can specify which one from the invokers.
WDYT?
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.
@chrisronline Actually this won't work. I tried passing it here:
return monitoringClusters(undefined, undefined, CODE_PATHS); |
monitoringClusters
executes somewhere else before hand (probably in a routeInit
), which sets the once variable before we can check the param. This means I would have to do some extra logic to determine the "real" origin (not as clean as the one liner above)
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!
@elasticmachine merge upstream |
Hold up on merging this. I'm testing this again and seeing some unexpected behavior. I'll update when I know more |
@@ -69,9 +69,9 @@ export function monitoringClustersProvider($injector) { | |||
if (Legacy.shims.isCloud) { | |||
return Promise.resolve(); | |||
} | |||
|
|||
const ccs = window.location.hash.split('?')[0] === '#/home' ? 'ccs' : 'single'; |
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.
Instead of this, why don't we just get the globalState
and add globalState.ccs
directly? The prefixIndexPattern
code is designed to accept a ccs
string like this
💚 Build SucceededMetrics [docs]async chunks size
History
To update your PR or re-run it, just comment with: |
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!
* fixed internal monitoring check * Added range filter * Added single vs ccs condtion * Fixed spelling * Passing global state ccs Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
* master: (23 commits) Table visualization renderer (elastic#79455) Migrate Jest JUnit reporter to TS (elastic#79919) store sorted bundleRefExportIds (elastic#80011) update chromedriver dependency to 86.0.0 (elastic#79972) [Security Solution][Case] Fix bug when changing connectors (elastic#80002) [kbn/std] add observable helpers to aid with rxjs 7 upgrade (elastic#79752) [Security Solution][Resolver] Pill numbers in compact notation (elastic#80038) [Logs UI] Sync logs timerange with wider Kibana (elastic#79444) [DOCS] Adds quick start (elastic#78822) [Ingest Manager]Fix ingest manager UI renaming (elastic#80036) [Monitoring] Fixed internal monitoring check (elastic#79241) [Security Solution][Exception Modal] Removes list operators in exception modal for EQL rules (elastic#79871) Update development documentation about REST API best practices (elastic#80009) [Monitoring] Improve indices loading against larger metricbeat-* indices (elastic#79190) [CI] Move kibana build dir outside of repo for functional tests (elastic#80018) [kbn/optimizer] bump low or add missing limits when updating automatically (elastic#80013) [Enterprise Search] Added a Credentials page to App Search (elastic#79749) [DOCS] Canvas refresh for 7.10 (elastic#80017) [TSVB] Add ignore global filters to series options (elastic#79337) Remove this check (elastic#79202) ...
* fixed internal monitoring check * Added range filter * Added single vs ccs condtion * Fixed spelling * Passing global state ccs Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # x-pack/plugins/monitoring/public/services/clusters.js
Resolves #79135
This was due to the addition of
metricbeat-*
indices, since their_index
field does not have the-mb-
marker