From 797ece3e1e8517b894eaa6238fbf71e21b1ef24b Mon Sep 17 00:00:00 2001 From: Milton Hultgren Date: Thu, 17 Nov 2022 10:44:58 +0100 Subject: [PATCH] [Stack Monitoring] Use doc ID to deduplicate shard allocations (#143963) ## Summary This PR depends on Metricbeat changes introduced in this PR https://github.com/elastic/beats/pull/33457 or the Internal collection changes introduced in https://github.com/elastic/elasticsearch/pull/91153. The above PR fixes a bug that makes Metricbeat properly report on all the shards in an index, but our shard legend had the same kind of bug, it only displayed the first replica because it didn't account for the replica "id" in the function that tries to deduplicate the results. Since the above PR ensures we report each shard with a unique document ID, we change the Kibana code to use the document ID instead of trying to regenerate a unique ID. ### How to test Get the changes: `git fetch git@github.com:miltonhultgren/kibana.git sm-shard-allocations:sm-shard-allocations && git switch sm-shard-allocations` Start Elasticsearch, create an index like this: ``` PUT /some-index { "settings": { "index": { "number_of_shards": 3, "number_of_replicas": 3 } } } ``` Run Metricbeat with `xpack.enabled: true`. Visit the Index details page for `some-index` in the Stack Monitoring app and verify that on a single node cluster you have 12 shards, 9 of which are unassigned. Screenshot 2022-10-25 at 17 19 25 Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit b7debc839ff6923923fe9a41355bd0318e04cdca) --- x-pack/plugins/monitoring/common/types/es.ts | 1 + .../lib/elasticsearch/shards/get_shard_allocation.test.js | 5 ++++- .../server/lib/elasticsearch/shards/get_shard_allocation.ts | 5 ++--- .../monitoring/server/lib/logstash/get_node_info.test.ts | 1 + 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/monitoring/common/types/es.ts b/x-pack/plugins/monitoring/common/types/es.ts index 977753de42d97..013d504fa8a73 100644 --- a/x-pack/plugins/monitoring/common/types/es.ts +++ b/x-pack/plugins/monitoring/common/types/es.ts @@ -17,6 +17,7 @@ export interface ElasticsearchResponse { } export interface ElasticsearchResponseHit { + _id: string; _index: string; _source: ElasticsearchSource; inner_hits?: { diff --git a/x-pack/plugins/monitoring/server/lib/elasticsearch/shards/get_shard_allocation.test.js b/x-pack/plugins/monitoring/server/lib/elasticsearch/shards/get_shard_allocation.test.js index 9c778553ca1a1..a89934d386742 100644 --- a/x-pack/plugins/monitoring/server/lib/elasticsearch/shards/get_shard_allocation.test.js +++ b/x-pack/plugins/monitoring/server/lib/elasticsearch/shards/get_shard_allocation.test.js @@ -37,8 +37,11 @@ describe('get_shard_allocation', () => { describe('handleResponse', () => { it('deduplicates shards', () => { const nextTimestamp = '2018-07-06T00:00:01.259Z'; - const hits = shards.map((shard) => { + const hits = shards.map((shard, index) => { return { + _id: `STATE_UUID:${shard.node}:${shard.index}:s${shard.shard}:${ + shard.primary ? 'p' : `r${index}` + }`, _source: { ...exampleShardSource, shard, diff --git a/x-pack/plugins/monitoring/server/lib/elasticsearch/shards/get_shard_allocation.ts b/x-pack/plugins/monitoring/server/lib/elasticsearch/shards/get_shard_allocation.ts index ab4fc1286d567..dd61ad4e1e59d 100644 --- a/x-pack/plugins/monitoring/server/lib/elasticsearch/shards/get_shard_allocation.ts +++ b/x-pack/plugins/monitoring/server/lib/elasticsearch/shards/get_shard_allocation.ts @@ -36,9 +36,8 @@ export function handleResponse(response: ElasticsearchResponse) { mbShard?.shard?.relocating_node?.id ?? legacyShard?.relocating_node ?? null; const node = mbShard?.node?.id ?? legacyShard?.node; // note: if the request is for a node, then it's enough to deduplicate without primary, but for indices it displays both - const shardId = `${index}-${shardNumber}-${primary}-${relocatingNode}-${node}`; - if (!uniqueShards.has(shardId)) { + if (!uniqueShards.has(hit._id)) { // @ts-ignore shards.push({ index, @@ -48,7 +47,7 @@ export function handleResponse(response: ElasticsearchResponse) { shard: shardNumber, state: legacyShard?.state ?? mbShard?.shard?.state, }); - uniqueShards.add(shardId); + uniqueShards.add(hit._id); } } diff --git a/x-pack/plugins/monitoring/server/lib/logstash/get_node_info.test.ts b/x-pack/plugins/monitoring/server/lib/logstash/get_node_info.test.ts index a43eb9a7cd09f..50410f4ef0ea5 100644 --- a/x-pack/plugins/monitoring/server/lib/logstash/get_node_info.test.ts +++ b/x-pack/plugins/monitoring/server/lib/logstash/get_node_info.test.ts @@ -33,6 +33,7 @@ jest.mock('../../static_globals', () => ({ // deletes, adds, or updates the properties based on a default object function createResponseObjHit(params?: HitParams[]): ElasticsearchResponseHit { const defaultResponseObj: ElasticsearchResponseHit = { + _id: '123123a', _index: 'index', _source: { cluster_uuid: '123',