Skip to content

Commit

Permalink
[Stack Monitoring] Do not add empty include array when retrieving log…
Browse files Browse the repository at this point in the history
…stash pipelines (#202039)

## Summary

This PR fixes the query that retrieves Logstash pipeline stats and
metrics to be displayed in Stack Monitoring. The problem with the
existing query is that a `terms` aggregation contains an empty
`"include": []` filtering array, but that has the nasty effect of
excluding everything and not returning anything, hence the screen is
empty.

The fix is to only add the `include` array if it's not empty. The PR
also fixes a wrong field name used in a cardinality aggregation:
`logstash.node.stats.logstash.uuid` instead of
`logstash_stats.logstash.uuid`

Closes #202020

### Checklist

- [X] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.

---------

Co-authored-by: Chris Earle <[email protected]>
(cherry picked from commit b7e46bd)
  • Loading branch information
consulthys committed Nov 28, 2024
1 parent a5f86f1 commit 818dd36
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 26 deletions.

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
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
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 818dd36

Please sign in to comment.