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

Deprecate Monitoring Settings #79499

Merged
merged 24 commits into from
Oct 20, 2021

Conversation

jbaiera
Copy link
Member

@jbaiera jbaiera commented Oct 19, 2021

This PR deprecates all monitoring settings as well as adds deprecation info entries for each setting.

Collecting and shipping monitoring data using the Monitoring plugin will be deprecated in 7.16 and will be removed at some point in the 8.x line after sufficient wait time. The recommended approach for collecting and shipping monitoring data going forward is to use Metricbeat. The recommended approach for alerting is Kibana alerting.

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

Copy link
Contributor

@danhermann danhermann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once you track down all the expectWarnings!

@jbaiera
Copy link
Member Author

jbaiera commented Oct 19, 2021

@elasticmachine update branch

@sethmlarson sethmlarson added the Team:Clients Meta label for clients team label Oct 19, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/clients-team (Team:Clients)

@jakelandis
Copy link
Contributor

Test fixes LGTM.

Spoke with Jimmy and the test code removed is/was dead. Monitoring was enabled for testing but never actually tested anything. With the new deprecation some other tests tripped over the fact that a setting was deprecated. So the test code that set the deprecated setting was removed since it was never actually used to test anything.

@jbaiera
Copy link
Member Author

jbaiera commented Oct 19, 2021

@elasticmachine run elasticsearch-ci/bwc

@jbaiera jbaiera merged commit 12d10b0 into elastic:master Oct 20, 2021
jbaiera added a commit that referenced this pull request Oct 20, 2021
This PR deprecates all monitoring settings as well as adds deprecation info entries for each setting.

Collecting and shipping monitoring data using the Monitoring plugin will be deprecated in 7.16 and will be removed at some point in the 8.x line after sufficient wait time. The recommended approach for collecting and shipping monitoring data going forward is to use Metricbeat. The recommended approach for alerting is Kibana alerting.

Backport of #79499
@jbaiera jbaiera deleted the monitoring-deprecate-settings branch October 20, 2021 15:35
@matschaffer
Copy link
Contributor

@jbaiera @jasonrhodes I was poking around ES source for the 8.0 compatibility testing and I noticed that this deprecation warning is still in place. Should I open an issue to revert this change?

@jakelandis
Copy link
Contributor

@matschaffer - can you be more specific in which deprecations you noticed for which workflows ? Legacy monitoring is deprecated so those should emit warnings (except on Cloud) and metricbeat/Fleet based monitoring should not emit warnings.

@jasonrhodes
Copy link
Member

Right, what @jakelandis said -- for a long time now, Internal Collection has been "deprecated" and Metricbeat collection (standalone) has been "recommended". There was a plan at one point to use 8.0 to remove Internal Collection and leave only metricbeat and agent/package collection, which is what we decided to shy away from.

Instead, we decided to keep everything as-is: Internal Collection is deprecated but available for the foreseeable future. Standalone Metricbeat collection is recommended. Packages will become recommended when they are available and stable, at which point we will deprecate standalone metricbeat collection as well. Then at some glorious point in the far future, both Internal and Standalone Metricbeat collections will be removed and only packages will remain (and whatever comes after packages, likely 😆 )

@matschaffer
Copy link
Contributor

@jakelandis sorry guess I should have been more specific, but basically what @jasonrhodes said. Since we're keeping everything as-is for now I was wondering if we should remove the deprecation warnings from ES code as well.

I think where I noticed it was when running ES (via ./gradlew run -Drun.license_type=trial -Dtests.es.xpack.monitoring.collection.enabled=true -Dtest.es.xpack.monitoring.exporters.id0.type=local) and kibana locally (via yarn start), I can see kibana checking & logging this:

[2022-01-05T12:17:26.533+09:00][INFO ][elasticsearch.deprecation] Elasticsearch deprecation: 299 Elasticsearch-8.1.0-SNAPSHOT-1e570620bf06f0b5b41ff88989bd679a6bce8ae2 "[xpack.monitoring.collection.enabled] setting was deprecated in Elasticsearch and will be removed in a future release! See the breaking changes documentation for the next major version."
Origin:kibana
Stack trace:
    at Cluster.getSettings (/Users/matschaffer/elastic/kibana/node_modules/@elastic/elasticsearch/src/api/api/cluster.ts:198:12)
    at isInlineScriptingEnabled (/Users/matschaffer/elastic/kibana/src/core/server/elasticsearch/is_scripting_enabled.ts:18:30)
    at ElasticsearchService.start (/Users/matschaffer/elastic/kibana/src/core/server/elasticsearch/elasticsearch_service.ts:121:32)
    at Server.start (/Users/matschaffer/elastic/kibana/src/core/server/server.ts:297:32)
    at Root.start (/Users/matschaffer/elastic/kibana/src/core/server/root/index.ts:62:14)
    at bootstrap (/Users/matschaffer/elastic/kibana/src/core/server/bootstrap.ts:102:5)
    at Command.<anonymous> (/Users/matschaffer/elastic/kibana/src/cli/serve/serve.js:244:5)
Query:
200
GET /_cluster/settings?include_defaults=true&flat_settings=true

I guess it's a question of how far ahead we want to show the warnings and balancing the log noise.

We have some internal threads like the "[TTL: 3 days] 7.17 Reducing deprecations from Elasticsearch" email that went to the kibana contributors list. Not sure where to balance warning the ES/kibana operator vs too much log noise.

@jakelandis
Copy link
Contributor

That command line is setting the deprecated settings and when Kibana is updating/reading settings the deprecation is emitted. In a new cluster these warnings should not be emitted and if Metric Beat monitoring is used these should also not emitted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants