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

[Monitoring] Introduce deprecation warning for shipping monitoring data to the production cluster #51718

Closed
cachedout opened this issue Nov 26, 2019 · 10 comments · Fixed by #72020
Assignees
Labels
Team:Monitoring Stack Monitoring team

Comments

@cachedout
Copy link
Contributor

cachedout commented Nov 26, 2019

This issue is a request for Kibana to add a deprecation log entry in 7.6 for users who are shipping monitoring data to the production cluster which urges them to switch to using Metricbeat monitoring in order to be prepared for 8.0.

@igoristic is this something you could take on?

cc: @elastic/stack-monitoring

@cachedout cachedout added the Team:Monitoring Stack Monitoring team label Nov 26, 2019
@cachedout
Copy link
Contributor Author

Ping @igoristic. Could you please see if this is something you could take on? Thanks!

@igoristic igoristic self-assigned this Dec 4, 2019
@igoristic
Copy link
Contributor

@cachedout @chrisronline Which config settings should trigger the depreciation log?

Is it just xpack.monitoring.collection.enabled or is it also all the setting related to it: https://www.elastic.co/guide/en/elasticsearch/reference/7.5/monitoring-settings.html#general-monitoring-settings ?

@chrisronline
Copy link
Contributor

@igoristic None of the existing settings are deprecated.

What we need to add is something telling users to switch to using these new configs or using Metricbeat as the collector and shipper of monitoring data.

@igoristic
Copy link
Contributor

@chrisronline For some reason I though that "Internal Monitoring" along with all their config settings are getting deprecated by 8.0

So, if none of the existing "Internal Monitoring" settings are actuating getting deprecated maybe then "depreciation" is a wrong term here? Maybe this should be more of a warning/suggestion instead?

I think it might make it kind of confusing for the customer if they are seeing the depreciation warning even-though they are already using Metricbeat. They might think that they still need to change/update something etc. (unless we word it as a general "If you're using...").

So, maybe just form UX point of view it's worth implementing a "smart" detection to see if they are already using Metricbeat to not show anything, and only show the depreciation warning if we detect something like xpack.monitoring.collection.enabled

My motivation for this approach is based on: https://github.com/elastic/kibana/blob/master/x-pack/legacy/plugins/monitoring/deprecations.js#L26

@chrisronline
Copy link
Contributor

Yea, it's tricky.

The "deprecated settings" in this case is really the configured exporters at the ES level.

Kibana only knows to send monitoring data to elasticsearch.hosts, but this configuration isn't exclusively for monitoring purposes so we can't do anything about that.

I think you're right - we might need to take a different approach with this one.

We only really care about .monitoring-kibana-* indices for this particular ticket. If they do have .monitoring-kibana-{version}-mb-* indices, they are all set and do not need to do anything. If they do not (and only have .monitoring-kibana-{version}-{date} indices), then they need to see some kind of warning that they need to either use Metricbeat to collect/ship monitoring data, or use the new configs we are adding

I don't know if we can use the typical deprecated channels, but we should try because those are where users are used to looking. We need to give users a chance of ensuring a smooth upgrade path in the 7.x timeline

@igoristic
Copy link
Contributor

igoristic commented Jan 6, 2020

We only really care about .monitoring-kibana-* indices for this particular ticket. If they do have .monitoring-kibana-{version}-mb-* indices, they are all set and do not need to do anything. If they do not (and only have .monitoring-kibana-{version}-{date} indices), then they need to see some kind of warning that they need to either use Metricbeat to collect/ship monitoring data, or use the new configs we are adding

This is exactly what I need. Thank you Chris!

We can include this where we have our current depreciation in the code, and also (like you mentioned) other streams (docs/release-notes)

@chrisronline chrisronline changed the title [Monitoring] Introduce deprecation warning for internal collection [Monitoring] Introduce deprecation warning for shipping monitoring data to the production cluster Jan 13, 2020
@sgrodzicki sgrodzicki added this to the Stack Monitoring UI 7.9 milestone Jun 23, 2020
@igoristic
Copy link
Contributor

igoristic commented Jul 15, 2020

@chrisronline I'm struggling to figure out how we can "truly" detect the use of "Internal Monitoring". My original approach of looking for a -mb- tag in the .monitoring-* index won't work, because you can still technically have both type of indices right after upgrading.

For my upgrading environment I kept the ES's /data folder and just did the upgrade to the one node (with the build that has the deprecation logic). I started metricbeat monitoring and was still seeing the error, simply because _cat/indices still showed monitoring indices without the -mb- tag (but, also along with the indices that did have the metricbeat tag)

@chrisronline
Copy link
Contributor

We do something similar with the migration wizard where we look back a limited amount of time, 30 seconds to be specific. If we see both -mb- and non -mb- indices, we conclude they might be in an upgrade scenario and the UI reflects that, but this is extremely temporary as it should only last thirty seconds if the user properly disabled internal collection. I think we handle it the same way for this task by having three states:

  1. User is entirely on internal collection and the messaging needs to indicate this
  2. User is partially on Metricbeat and partially on internal collection (In this state, we may not know if they did indeed disable internal collection yet so the messaging needs to reflect this)
  3. User is entirely on Metricbeat collection

@igoristic
Copy link
Contributor

Ah cool! Thanks! Don't know how I missed that. I actually ended up doing something similar, but instead, I'm just making an empty search query (on all .monitoring-* indices) then I look for the -mb- tag in hits._index

we look back a limited amount of time, 30 seconds to be specific. If we see both -mb- and non -mb- indices, we conclude they might be in an upgrade scenario

This might not work so well if they have their collection interval set to something more than 30s


This brings up a bigger concerns though, since they can still go back in time and eventually get documents from those none -mb- indices, which might break something if there's schema changes or anything like that after the actual depreciation. Also, ES might not manage them anymore (but probably a separate issue)

I think it makes more sense to just DELETE all none -mb- indices as part of migration

I might be overthinking this though or describing edge cases (that never happen) 🤷

@chrisronline
Copy link
Contributor

This brings up a bigger concerns though, since they can still go back in time and eventually get documents from those none -mb- indices, which might break something if there's schema changes or anything like that after the actual depreciation. Also, ES might not manage them anymore (but probably a separate issue)

This should be manageable since we won't make schema changes in a minor, and when we go to 8.0, we will provision a new index .monitoring-{product}-8* and if we know that is a breaking change, we can change the index to only look for -8* indices.

I think it makes more sense to just DELETE all none -mb- indices as part of migration

But they shouldn't have too. The data in each index should be identical (and we have the parity tests to ensure this)

This might not work so well if they have their collection interval set to something more than 30s

This is definitely true and we should file a bug to handle this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Monitoring Stack Monitoring team
Projects
No open projects
4 participants