Skip to content

Commit

Permalink
[8.x] [Stack Monitoring] Do not add empty include array when retrievi…
Browse files Browse the repository at this point in the history 
…ng logstash pipelines (#202039) (#202101)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Stack Monitoring] Do not add empty include array when retrieving
logstash pipelines
(#202039)](#202039)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Valentin
Crettaz","email":"valentin.crettaz@elastic.co"},"sourceCommit":{"committedDate":"2024-11-28T04:05:51Z","message":"[Stack
Monitoring] Do not add empty include array when retrieving logstash
pipelines (#202039)\n\n## Summary\r\n\r\nThis PR fixes the query that
retrieves Logstash pipeline stats and\r\nmetrics to be displayed in
Stack Monitoring. The problem with the\r\nexisting query is that a
`terms` aggregation contains an empty\r\n`\"include\": []` filtering
array, but that has the nasty effect of\r\nexcluding everything and not
returning anything, hence the screen is\r\nempty.\r\n\r\nThe fix is to
only add the `include` array if it's not empty. The PR\r\nalso fixes a
wrong field name used in a cardinality
aggregation:\r\n`logstash.node.stats.logstash.uuid` instead
of\r\n`logstash_stats.logstash.uuid`\r\n\r\nCloses
https://github.com/elastic/kibana/issues/202020\r\n\r\n###
Checklist\r\n\r\n- [X] This was checked for breaking HTTP API changes,
and any breaking\r\nchanges have been approved by the breaking-change
committee. The\r\n`release_note:breaking` label should be applied in
these situations.\r\n\r\n---------\r\n\r\nCo-authored-by: Chris Earle
<chris.earle@elastic.co>","sha":"b7e46bdae9da7ac6ea7346cd3450597796995eb5","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["backport","Team:Monitoring","release_note:skip","Feature:Stack
Monitoring","v9.0.0","backport:version","v8.17.0","v8.18.0","v8.16.2","v8.15.6"],"title":"[Stack
Monitoring] Do not add empty include array when retrieving logstash
pipelines","number":202039,"url":"https://github.com/elastic/kibana/pull/202039","mergeCommit":{"message":"[Stack
Monitoring] Do not add empty include array when retrieving logstash
pipelines (#202039)\n\n## Summary\r\n\r\nThis PR fixes the query that
retrieves Logstash pipeline stats and\r\nmetrics to be displayed in
Stack Monitoring. The problem with the\r\nexisting query is that a
`terms` aggregation contains an empty\r\n`\"include\": []` filtering
array, but that has the nasty effect of\r\nexcluding everything and not
returning anything, hence the screen is\r\nempty.\r\n\r\nThe fix is to
only add the `include` array if it's not empty. The PR\r\nalso fixes a
wrong field name used in a cardinality
aggregation:\r\n`logstash.node.stats.logstash.uuid` instead
of\r\n`logstash_stats.logstash.uuid`\r\n\r\nCloses
https://github.com/elastic/kibana/issues/202020\r\n\r\n###
Checklist\r\n\r\n- [X] This was checked for breaking HTTP API changes,
and any breaking\r\nchanges have been approved by the breaking-change
committee. The\r\n`release_note:breaking` label should be applied in
these situations.\r\n\r\n---------\r\n\r\nCo-authored-by: Chris Earle
<chris.earle@elastic.co>","sha":"b7e46bdae9da7ac6ea7346cd3450597796995eb5"}},"sourceBranch":"main","suggestedTargetBranches":["8.17","8.x","8.16","8.15"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/202039","number":202039,"mergeCommit":{"message":"[Stack
Monitoring] Do not add empty include array when retrieving logstash
pipelines (#202039)\n\n## Summary\r\n\r\nThis PR fixes the query that
retrieves Logstash pipeline stats and\r\nmetrics to be displayed in
Stack Monitoring. The problem with the\r\nexisting query is that a
`terms` aggregation contains an empty\r\n`\"include\": []` filtering
array, but that has the nasty effect of\r\nexcluding everything and not
returning anything, hence the screen is\r\nempty.\r\n\r\nThe fix is to
only add the `include` array if it's not empty. The PR\r\nalso fixes a
wrong field name used in a cardinality
aggregation:\r\n`logstash.node.stats.logstash.uuid` instead
of\r\n`logstash_stats.logstash.uuid`\r\n\r\nCloses
https://github.com/elastic/kibana/issues/202020\r\n\r\n###
Checklist\r\n\r\n- [X] This was checked for breaking HTTP API changes,
and any breaking\r\nchanges have been approved by the breaking-change
committee. The\r\n`release_note:breaking` label should be applied in
these situations.\r\n\r\n---------\r\n\r\nCo-authored-by: Chris Earle
<chris.earle@elastic.co>","sha":"b7e46bdae9da7ac6ea7346cd3450597796995eb5"}},{"branch":"8.17","label":"v8.17.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.16","label":"v8.16.2","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.15","label":"v8.15.6","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Valentin Crettaz <valentin.crettaz@elastic.co>
  • Loading branch information
@kibanamachine @consulthys
kibanamachine and consulthys authored Nov 28, 2024
1 parent 561ed65 commit 6c69bed
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 26 deletions.
4 changes: 2 additions & 2 deletions x-pack/plugins/monitoring/server/lib/metrics/__snapshots__/metrics.test.js.snap
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 10 additions & 15 deletions x-pack/plugins/monitoring/server/lib/metrics/logstash/classes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@

/* eslint-disable max-classes-per-file */

import _ from 'lodash';
import { i18n } from '@kbn/i18n';
import { ClusterMetric, Metric, MetricOptions } from '../classes';
import { LARGE_FLOAT } from '../../../../common/formatting';
import _ from 'lodash';
import { NORMALIZED_DERIVATIVE_UNIT } from '../../../../common/constants';
import { LARGE_FLOAT } from '../../../../common/formatting';
import { ClusterMetric, Metric, MetricOptions } from '../classes';

const msTimeUnitLabel = i18n.translate('xpack.monitoring.metrics.logstash.msTimeUnitLabel', {
defaultMessage: 'ms',
Expand Down Expand Up @@ -411,7 +411,7 @@ export class LogstashPipelineThroughputMetric extends LogstashMetric {

type LogstashPipelineNodeCountMetricOptions = Pick<
MetricOptions,
'field' | 'label' | 'description' | 'format' | 'units'
'field' | 'label' | 'mbField' | 'description' | 'format' | 'units'
> &
Partial<Pick<MetricOptions, 'uuidField'>>;

Expand All @@ -427,15 +427,10 @@ export class LogstashPipelineNodeCountMetric extends LogstashMetric {
}: {
pageOfPipelines: Array<{ id: string }>;
}) => {
const termAggExtras: {
include: string[];
} = {
include: [],
};
const include: string[] | undefined = pageOfPipelines?.length
? pageOfPipelines.map((pipeline) => pipeline.id)
: undefined;

if (pageOfPipelines) {
termAggExtras.include = pageOfPipelines.map((pipeline) => pipeline.id);
}
return {
pipelines_nested: {
nested: {
Expand All @@ -446,7 +441,7 @@ export class LogstashPipelineNodeCountMetric extends LogstashMetric {
terms: {
field: 'logstash_stats.pipelines.id',
size: 1000,
...termAggExtras,
include,
},
aggs: {
to_root: {
Expand All @@ -472,15 +467,15 @@ export class LogstashPipelineNodeCountMetric extends LogstashMetric {
terms: {
field: 'logstash.node.stats.pipelines.id',
size: 1000,
...termAggExtras,
include,
},
aggs: {
to_root: {
reverse_nested: {},
aggs: {
node_count: {
cardinality: {
field: this.field,
field: this.mbField,
},
},
},
Expand Down
20 changes: 11 additions & 9 deletions x-pack/plugins/monitoring/server/lib/metrics/logstash/metrics.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,23 @@
*/

import { i18n } from '@kbn/i18n';
import {
LARGE_ABBREVIATED,
LARGE_BYTES,
LARGE_FLOAT,
SMALL_BYTES,
} from '../../../../common/formatting';
import { QuotaMetric } from '../classes';
import {
LogstashEventsRateClusterMetric,
LogstashEventsLatencyClusterMetric,
LogstashEventsRateMetric,
LogstashEventsLatencyMetric,
LogstashEventsRateClusterMetric,
LogstashEventsRateMetric,
LogstashMetric,
LogstashPipelineNodeCountMetric,
LogstashPipelineQueueSizeMetric,
LogstashPipelineThroughputMetric,
LogstashPipelineNodeCountMetric,
} from './classes';
import {
LARGE_FLOAT,
LARGE_BYTES,
SMALL_BYTES,
LARGE_ABBREVIATED,
} from '../../../../common/formatting';

const instanceSystemLoadTitle = i18n.translate(
'xpack.monitoring.metrics.logstash.systemLoadTitle',
Expand Down Expand Up @@ -451,6 +451,7 @@ export const metrics = {
logstash_cluster_pipeline_nodes_count: new LogstashPipelineNodeCountMetric({
field: 'logstash_stats.logstash.uuid',
label: pipelineNodeCountLabel,
mbField: 'logstash.node.stats.logstash.uuid',
description: pipelineNodeCountDescription,
format: LARGE_FLOAT,
units: '',
Expand All @@ -459,6 +460,7 @@ export const metrics = {
uuidField: 'logstash_stats.logstash.uuid', // TODO: add comment explaining why
field: 'logstash_stats.logstash.uuid',
label: pipelineNodeCountLabel,
mbField: 'logstash.node.stats.logstash.uuid',
description: pipelineNodeCountDescription,
format: LARGE_FLOAT,
units: '',
Expand Down

0 comments on commit 6c69bed

Please sign in to comment.