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

Disable monitoring in ML multinode tests #55461

Conversation

williamrandolph
Copy link
Contributor

Removing the deprecated "xpack.monitoring.enabled" setting introduced log spam and potentially some failures in ML tests. It's possible to use a different, non-deprecated setting to disable monitoring, so we do that here.

Relates #55420
Relates #54816

Removing the deprecated "xpack.monitoring.enabled" setting introduced
log spam and potentially some failures in ML tests. It's possible to use
a different, non-deprecated setting to disable monitoring, so we do that
here.
@williamrandolph williamrandolph added >test Issues or PRs that are addressing/adding tests :Core/Infra/Core Core issues without another label v8.0.0 v7.8.0 labels Apr 20, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/Core)

@droberts195
Copy link
Contributor

There is also a place in Java code where ML is disabling monitoring using the deprecated setting:

Should that also be flipped over to using xpack.monitoring.elasticsearch.collection.enabled instead too?

In fact, if you search for usages of XPackSettings.MONITORING_ENABLED there are still about ten places in tests that use it to disable monitoring through Java code. Should they all be switched over?

@williamrandolph
Copy link
Contributor Author

@droberts195 I was thinking of doing those in a follow-up PR but I'll go ahead and handle them here.

Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

I was thinking of doing those in a follow-up PR

@williamrandolph if you think there's a risk of breaking other things then a follow-up PR is fine with me.

LGTM for the changes so far.

Copy link
Member

@benwtrent benwtrent left a comment

Choose a reason for hiding this comment

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

seems good to me :)

@williamrandolph williamrandolph merged commit 7a039aa into elastic:master Apr 20, 2020
williamrandolph added a commit that referenced this pull request Apr 20, 2020
Removing the deprecated "xpack.monitoring.enabled" setting introduced
log spam and potentially some failures in ML tests. It's possible to use
a different, non-deprecated setting to disable monitoring, so we do that
here.
@williamrandolph
Copy link
Contributor Author

Backported to 7.x: 7817948

@williamrandolph williamrandolph deleted the re-disable-monitoring-in-ml-tests branch May 23, 2022 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label >test Issues or PRs that are addressing/adding tests v7.8.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants