Index cluster.id, node.id, and node.name in elasticsearch/node_stats metricset #9168
Conversation
ycombinator
added
review
Metricbeat
| @@ -140,6 +141,10 @@ func eventsMapping(r mb.ReporterV2, content []byte) error { | |||
| errs = append(errs, event.Error) | |||
| } | |||
|
|
|||
| name, _ := event.MetricSetFields.GetValue("name") | |||
| event.ModuleFields.Put("node.name", name.(string)) | |||
There was a problem hiding this comment.
We should probably check the type conversion here? Also what happens if above returns and error and name does not exist? Will name be nil?
There was a problem hiding this comment.
I didn't add a check here because it will get caught in the schema conversion itself.
There was a problem hiding this comment.
I see. Agree that it should not break here but what if someone modifies the other part of the code and does not thing about this part? I normally prefer to be on the safe side.
There was a problem hiding this comment.
Fixed in bcd8191.
|
CI test failure is valid. Fixing... |
|
@ruflin I added error handling and fixed the CI test failure. Ready for your 👀 again. Thanks! |
ycombinator
force-pushed
the
metricbeat-elasticsearch-node-stats-node-id-cluster-id-name
branch
from
bcd8191
to
09e940a
Compare
| release: beta | ||
| fields: | ||
| - name: indices | ||
| type: group | ||
| - name: id |
There was a problem hiding this comment.
I would assume this is now a duplicated field as it's also in node metricset? I general I would recommend to configure "shared" fields on the module level.
|
jenkins, test this |
1 similar comment
|
jenkins, test this |
|
@ruflin Moved node fields to module level. Ready for your 👀 again. Thanks! |
| release: beta | ||
| fields: | ||
| - name: indices | ||
| - name: stats |
There was a problem hiding this comment.
Not sure why you introduce the mapping here (was it for the other fields)? In the end both should work.
There was a problem hiding this comment.
Ugh, this change should've been reverted when I moved the node fields to the module level. Sorry about that. Fixing now...
There was a problem hiding this comment.
Reverted in b5912d8.
ycombinator
force-pushed
the
metricbeat-elasticsearch-node-stats-node-id-cluster-id-name
branch
from
614ca9a
to
b5912d8
Compare
…metricset (elastic#9168) This PR teaches the `elasticsearch/node_stats` metricset to index the Elasticsearch `cluster_uuid` as the module-level `cluster.id` field, as well as the node ID and node `name` fields as the metricset-level `node.id` and `node.name` fields, respectively. (cherry picked from commit 9410148)
ycombinator
removed
the
needs_backport
… elasticsearch/node_stats metricset (#9255) Cherry-pick of PR #9168 to 6.x branch. Original message: This PR teaches the `elasticsearch/node_stats` metricset to index the Elasticsearch `cluster_uuid` as the module-level `cluster.id` field, as well as the node ID and node `name` fields as the metricset-level `node.id` and `node.name` fields, respectively.
ycombinator
deleted the
metricbeat-elasticsearch-node-stats-node-id-cluster-id-name
branch
This PR teaches the
elasticsearch/node_statsmetricset to index the Elasticsearchcluster_uuidas the module-levelcluster.idfield, as well as the node ID and nodenamefields as the metricset-levelnode.idandnode.namefields, respectively.