elasticsearch module - xpack enabled parity #29548

klacabane · 2021-12-20T14:18:23Z

Summary

Fix drifts from 7.16 in elasticsearch module's documents when xpack.enabled is set.
Fix incomplete node property for ml_job and shard metricsets
Fix shard's cluster state property path to be consistent with cluster_stats

elasticmachine · 2021-12-20T14:18:25Z

Pinging @elastic/integrations (Team:Integrations)

elasticmachine · 2021-12-20T14:18:25Z

Pinging @elastic/stack-monitoring (Stack monitoring)

klacabane · 2021-12-20T14:18:51Z

metricbeat/module/elasticsearch/index_recovery/index_recovery.go

 	}
 	if err := base.Module().UnpackConfig(&config); err != nil {
 		return nil, err
 	}

 	localRecoveryPath := recoveryPath
-	if config.ActiveOnly {
+	if !config.XPack && config.ActiveOnly {


See https://github.com/elastic/beats/blame/7.16/metricbeat/module/elasticsearch/index_recovery/index_recovery.go#L55

I think that unless something is specifically broken, we shouldn't need to add this check back.

Are we fine flipping the ActiveOnly default to false then ?

Yes because the xpack flag will eventually dissapear. But users must still explicitly check to true in the case of active_only.

klacabane · 2021-12-20T14:24:13Z

metricbeat/module/elasticsearch/ml_job/data.go

@@ -81,7 +81,6 @@ func eventsMapping(r mb.ReporterV2, info elasticsearch.Info, content []byte, isX
 		event.ModuleFields = common.MapStr{}
 		event.ModuleFields.Put("cluster.name", info.ClusterName)
 		event.ModuleFields.Put("cluster.id", info.ClusterID)
-		event.ModuleFields.Put("node.id", info.Name)


The problem here is that we set the name instead of ID which breaks some href in the UI because we usually redirect with the ID. 7.16 didn't have this property set at all so removing it from now but we should populate the node property correctly.

See https://github.com/elastic/beats/blob/7.16/metricbeat/module/elasticsearch/ml_job/data_xpack.go#L68

I don't see any reason to remove it. I actually don't remember why I added it but we can always cycle if necessary.

Removed it because we write a name where the consumer of these documents expect an ID. It currently breaks the stack monitoring UI because we build a link out of this ID to redirect to the node's details page, and the redirection fails since we can't find the node details from its name.
Can we push this and follow up with the appropriate fix that would populate the node.id and name ?

Populated the node object by parsing the ml_job response.
The conditional is there because the property is only available for opened jobs

Sorry, I meant to not remove it. I forgot the "not" 😅

elasticmachine · 2021-12-20T14:55:46Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2021-12-21T14:25:27.793+0000
Duration: 86 min 24 sec
Commit: e433ec8

Test stats 🧪

Test	Results
Failed	0
Passed	9394
Skipped	2432
Total	11826

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

sayden · 2021-12-21T13:00:06Z

metricbeat/module/elasticsearch/ml_job/data.go

@@ -81,7 +81,6 @@ func eventsMapping(r mb.ReporterV2, info elasticsearch.Info, content []byte, isX
 		event.ModuleFields = common.MapStr{}
 		event.ModuleFields.Put("cluster.name", info.ClusterName)
 		event.ModuleFields.Put("cluster.id", info.ClusterID)
-		event.ModuleFields.Put("node.id", info.Name)


Sorry, I meant to not remove it. I forgot the "not" 😅

sayden · 2021-12-21T13:03:11Z

metricbeat/module/elasticsearch/shard/data.go

@@ -72,7 +72,7 @@ func eventsMapping(r mb.ReporterV2, content []byte, isXpack bool) error {
 					ModuleFields: common.MapStr{},
 				}

-				event.ModuleFields.Put("cluster.state.id", stateData.StateID)
+				event.ModuleFields.Put("cluster.stats.state.state_uuid", stateData.StateID)


This is breaking change at the moment. Instead of removing the old field, add a new one duplicating the value of the current one.

At the same time, we need to add this new field in the corresponding fields.yml, this one: https://github.com/elastic/beats/blob/master/metricbeat/module/elasticsearch/cluster_stats/_meta/fields.yml

The run a mage fmt update at the root of metricbeat folder and you'll see a bunch of files changed.

Isn't this field already defined or am I looking at wrong place ?

It is already defined, yes. But that just mean "index it" maybe it was a leftover and we finally don't write into that field in the module.

sayden

LGTM

* elasticsearch module - xpack enabled parity * flip ActiveOnly default * add ml_job's node * lint * update shard cluster's state path * add back state.id to prevent breaking changes (cherry picked from commit 0bfa0a1)

* elasticsearch module - xpack enabled parity * flip ActiveOnly default * add ml_job's node * lint * update shard cluster's state path * add back state.id to prevent breaking changes (cherry picked from commit 0bfa0a1) Co-authored-by: Kevin Lacabane <[email protected]>

elasticsearch module - xpack enabled parity

50d7d4e

klacabane added Team:Integrations Label for the Integrations team Feature:Stack Monitoring v8.0.0 backport-v8.0.0 Automated backport with mergify v8.1.0 labels Dec 20, 2021

klacabane self-assigned this Dec 20, 2021

botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Dec 20, 2021

klacabane commented Dec 20, 2021

View reviewed changes

klacabane requested a review from sayden December 20, 2021 14:24

klacabane mentioned this pull request Dec 20, 2021

Stack monitoring.elasticsearch - 8.0 compatibility elastic/kibana#121282

Closed

3 tasks

flip ActiveOnly default

7a8478f

klacabane added 3 commits December 21, 2021 03:25

add ml_job's node

b4aafb1

lint

3939ab3

update shard cluster's state path

1fcecf8

sayden reviewed Dec 21, 2021

View reviewed changes

add back state.id to prevent breaking changes

e433ec8

klacabane mentioned this pull request Dec 21, 2021

[Stack monitoring] adapt ES queries to updated document structure elastic/kibana#121641

Merged

sayden approved these changes Dec 23, 2021

View reviewed changes

klacabane merged commit 0bfa0a1 into elastic:master Dec 23, 2021

mergify bot mentioned this pull request Dec 23, 2021

[8.0](backport #29548) elasticsearch module - xpack enabled parity #29596

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

elasticsearch module - xpack enabled parity #29548

elasticsearch module - xpack enabled parity #29548

klacabane commented Dec 20, 2021 •

edited

Loading

elasticmachine commented Dec 20, 2021

elasticmachine commented Dec 20, 2021

klacabane Dec 20, 2021

sayden Dec 20, 2021

klacabane Dec 20, 2021

sayden Dec 20, 2021

klacabane Dec 20, 2021

sayden Dec 20, 2021

klacabane Dec 20, 2021

klacabane Dec 21, 2021

sayden Dec 21, 2021

elasticmachine commented Dec 20, 2021 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

sayden Dec 21, 2021

sayden Dec 21, 2021

klacabane Dec 21, 2021

sayden Dec 23, 2021

sayden left a comment

elasticsearch module - xpack enabled parity #29548

elasticsearch module - xpack enabled parity #29548

Conversation

klacabane commented Dec 20, 2021 • edited Loading

Summary

elasticmachine commented Dec 20, 2021

elasticmachine commented Dec 20, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Dec 20, 2021 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sayden left a comment

Choose a reason for hiding this comment

klacabane commented Dec 20, 2021 •

edited

Loading

elasticmachine commented Dec 20, 2021 •

edited by jenkins-beats-ci bot

Loading