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.

Sign up for GitHub

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

Jump to bottom

[Elasticsearch-xpack] [Metricbeat] Fix elasticsearch-xpack error when Machine Learning is explicitly disabled #39990

Merged
merged 6 commits into from
Jul 9, 2024
Merged

[Elasticsearch-xpack] [Metricbeat] Fix elasticsearch-xpack error when Machine Learning is explicitly disabled #39990

merged 6 commits into from
Jul 9, 2024

Conversation

jennypavlova
Copy link
Member

@jennypavlova jennypavlova commented Jun 21, 2024 •
edited
Loading

Closes #28087

  • Bug

Proposed commit message

This PR fixes elasticsearch-xpack error when Machine Learning is explicitly disabled by checking if the Machine Learning is enabled

@jennypavlova jennypavlova self-assigned this Jun 21, 2024
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jun 21, 2024
@mergify Mergify
Copy link
Contributor

mergify bot commented Jun 21, 2024

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @jennypavlova? 🙏.
For such, you'll need to label your PR with:

  • The upcoming major version of the Elastic Stack
  • The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

klacabane
klacabane reviewed Jun 21, 2024
View reviewed changes
metricbeat/module/elasticsearch/ml_job/ml_job.go Outdated
@@ -72,3 +81,17 @@ func (m *MetricSet) Fetch(r mb.ReporterV2) error {

return eventsMapping(r, info, content, m.XPackEnabled)
}

func (m *MetricSet) checkMLAvailability() (message string, err error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we return boolean instead of a string ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, changed ✅

@jennypavlova jennypavlova added the Team:Monitoring Stack Monitoring team label Jun 21, 2024
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jun 21, 2024
@jennypavlova jennypavlova added needs_team Indicates that the issue/PR needs a Team:* label release-note:fix The content should be included as a fix backport-skip Skip notification from the automated backport with mergify labels Jun 21, 2024
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jun 21, 2024
@botelastic
Copy link

botelastic bot commented Jun 21, 2024

This pull request doesn't have a Team:<team> label.

jennypavlova
jennypavlova commented Jun 21, 2024
View reviewed changes
metricbeat/module/elasticsearch/ml_job/ml_job_test.go
package ml_job

import (
"io/ioutil"
Copy link
Member Author

Choose a reason for hiding this comment

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

I am not sure why I see these errors - the same import exists in other files 🤔

@jennypavlova
Copy link
Member Author

@klacabane Thank you for the review!
I fixed the comments and the test and don't see the error when xpack.ml.enabled is set to false.
I would need some help with the linting errors (I tried some formatters and tips I found but nothing worked so far) and testing with the actual ML job running locally (the case when the ML is enabled in elasticsearch - I am not sure what the expected log is in this case and if any additional configuration is needed).

@klacabane klacabane marked this pull request as ready for review July 8, 2024 09:23
@klacabane klacabane requested a review from a team as a code owner July 8, 2024 09:23
@klacabane klacabane merged commit 032a4cf into elastic:main Jul 9, 2024
17 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@klacabane klacabane klacabane approved these changes

Assignees

@jennypavlova jennypavlova

Labels
backport-skip Skip notification from the automated backport with mergify release-note:fix The content should be included as a fix Team:Monitoring Stack Monitoring team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Metricbeat monitoring module elasticsearch-xpack error when Machine Learning is explicitly disabled
2 participants
@jennypavlova @klacabane