-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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] "Internal Monitoring" deprecation warning #72020
[Monitoring] "Internal Monitoring" deprecation warning #72020
Conversation
Pinging @elastic/stack-monitoring (Team:Monitoring) |
…nal-monitoring-deprecation
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.
Looking good! Nice work so far! Left a few notes
}); | ||
}; | ||
|
||
export const showInternalMonitoringToast = ({ |
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.
Just curious, why did you decide to go with a toast instead of a EuiCallout
at the top of the page?
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 think it's less invasive. Also I feel like EuiCallout
is too close to the enter setup button which makes it appear as if they're somehow associated.
x-pack/plugins/monitoring/public/lib/internal_monitoring_toasts.tsx
Outdated
Show resolved
Hide resolved
|
||
<EuiSpacer /> | ||
<EuiLink | ||
href={`${ELASTIC_WEBSITE_URL}blog/external-collection-for-elastic-stack-monitoring-is-now-available-via-metricbeat`} |
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.
Love it!
x-pack/plugins/monitoring/public/lib/internal_monitoring_toasts.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/monitoring/public/lib/internal_monitoring_toasts.tsx
Outdated
Show resolved
Hide resolved
.../plugins/monitoring/server/routes/api/v1/elasticsearch_settings/check/internal_monitoring.ts
Outdated
Show resolved
Hide resolved
.../plugins/monitoring/server/routes/api/v1/elasticsearch_settings/check/internal_monitoring.ts
Outdated
Show resolved
Hide resolved
.../plugins/monitoring/server/routes/api/v1/elasticsearch_settings/check/internal_monitoring.ts
Outdated
Show resolved
Hide resolved
…nal-monitoring-deprecation
…nal-monitoring-deprecation
…nal-monitoring-deprecation
.../plugins/monitoring/server/routes/api/v1/elasticsearch_settings/check/internal_monitoring.ts
Outdated
Show resolved
Hide resolved
.../plugins/monitoring/server/routes/api/v1/elasticsearch_settings/check/internal_monitoring.ts
Outdated
Show resolved
Hide resolved
_index.includes('-mb-') | ||
).length; | ||
|
||
counts.legacyIndicesCount = hits.length - counts.mbIndicesCount; |
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.
These counts seem off to me.
I added this debug code:
console.log(hits.map((h) => h._index));
which results in:
[ '.monitoring-kibana-7-2020.07.21',
'.monitoring-kibana-7-2020.07.21',
'.monitoring-kibana-7-2020.07.21',
'.monitoring-kibana-7-2020.07.21',
'.monitoring-kibana-7-2020.07.21',
'.monitoring-kibana-7-2020.07.21',
'.monitoring-kibana-7-2020.07.21',
'.monitoring-kibana-7-2020.07.21',
'.monitoring-kibana-7-2020.07.21',
'.monitoring-kibana-7-2020.07.21',
'.monitoring-kibana-7-2020.07.21',
'.monitoring-kibana-7-2020.07.21',
'.monitoring-kibana-7-2020.07.21',
'.monitoring-kibana-7-2020.07.21',
'.monitoring-kibana-7-2020.07.21' ]
Maybe you can just do a query like:
POST .monitoring-es-*/_search
{
"size": 0,
"aggs": {
"types": {
"terms": {
"field": "_index",
"size": 10
}
}
}
}
…nal-monitoring-deprecation
…nal-monitoring-deprecation
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@igoristic It looks like the failing tests are from monitoring. Maybe something with these changes broke them? |
@chrisronline Thats what I thought, but I ran them locally and they all pass. Not sure what the problem is, since CI always shows a different failed test |
@elasticmachine merge upstream |
<p> | ||
{i18n.translate('xpack.monitoring.internalAndMetricbeatMonitoringToast.description', { | ||
defaultMessage: `It appears you are using both Metricbeat and "Legacy Collection" for Stack Monitoring. | ||
only Metricbeat type monitoring will be supported in the next major release (8.0.0). |
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.
What if we said: Monitoring will require Metricbeat collection in the next major version (8.0.0).
or maybe @gchaps can help here
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.
How about:
In 8.0.0, you must use Metricbeat to collect monitoring data.
…nal-monitoring-deprecation
💚 Build SucceededBuild metricsasync 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! Nice work!
Backport: |
* Internal Monitoring deprecation * Fixed type * Added if cloud logic * Fixed i18n test * Addressed code review feedback * Fixed types * Changed query * Added delay to fix a test * Fixed tests * Fixed text Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@igoristic we would need to backport just the bugfix to make it work for 7.9 |
Resolves: #51718
Detecting the type of
.monitoring-*
indices in their cluster to assume which type of method of monitoring their using.If they don't have any
-mb-
tags in their monitoring indices we show the following message:If for some reason they have both types, we instead show:
We don't show anything if they're on Cloud (for now)