[Infra UI] Add log rate to metrics on Waffle Map #23834
Conversation
This PR adds the log rate metric to the waffle map
💚 Build Succeeded |
💚 Build Succeeded |
| @@ -59,6 +59,10 @@ const METRIC_FORMATTERS: MetricFormatters = { | |||
| }, | |||
| [InfraMetricType.rx]: { formatter: InfraFormatterType.bits, template: '{{value}}/s' }, | |||
| [InfraMetricType.tx]: { formatter: InfraFormatterType.bits, template: '{{value}}/s' }, | |||
| [InfraMetricType.logRate]: { | |||
| formatter: InfraFormatterType.abbreviatedNumber, | |||
| template: '{{value}}/s', | |||
There was a problem hiding this comment.
Does the value really have the unit s? It depends on the histogram bucket size (AFAIK 1m on the waffle map right now). So we either use value with the same unit as the histogram bucket size or we use the normalized_value which should respect the 1s unit specified in the derivative agg, right?
I wonder if this also applies to the other rates like rx, which were already added previously?
There was a problem hiding this comment.
Yeah... I totally over looked the normalized value when I was setting this up originally. I just added a commit to fix this.
| @@ -24,6 +25,9 @@ export function createPartitionBodies( | |||
| const { sourceConfiguration }: InfraNodeRequestOptions = nodeOptions; | |||
| const bodies: InfraESMSearchBody[] = []; | |||
| const numberOfPartitions: number = Math.ceil(totalNodes / NODE_REQUEST_PARTITION_SIZE); | |||
| const indices = nodeOptions.metrics.every(m => m.type === InfraMetricType.logRate) | |||
| ? [sourceConfiguration.logAlias] | |||
| : [sourceConfiguration.logAlias, sourceConfiguration.metricAlias]; | |||
There was a problem hiding this comment.
What happens if the graphql request contains both a logRate metric and something like a cpu metric? From what I can observe, this leads to incorrect log rate calculations because it counts the docs from metric indices as well. 🤔
💚 Build Succeeded |
💔 Build Failed |
💚 Build Succeeded |
|
@weltenwort This is ready for a follow up review. I refactored the waffle map endpoint so it will only accept 1 metric instead of multiple. |
| .reduce((prev: InfraMetricInput[], argument: ArgumentNode) => { | ||
| return valueFromASTUntyped(argument.value); | ||
| }, []) | ||
| .pop(); |
There was a problem hiding this comment.
Does the above reduce return an array even though the metric argument is not an array anymore?
| path: info.variableValues.path as InfraPathInput[], | ||
| }; | ||
| } | ||
|
|
||
| const { path, metrics }: InfraPathsAndMetricOptions = info.fieldNodes.reduce(parseFieldNodes, { | ||
| const { path, metric }: InfraPathsAndMetricOptions = info.fieldNodes.reduce(parseFieldNodes, { |
There was a problem hiding this comment.
This code path seems to not work correctly. The following query leads to an exception:
query WaffleNodesQuery(
$sourceId: ID!
$timerange: InfraTimerangeInput!
$filterQuery: String
$path: [InfraPathInput!]!
) {
source(id: $sourceId) {
id
map(timerange: $timerange, filterQuery: $filterQuery) {
nodes(path: $path) {
path {
value
}
metric(metric: {type:cpu}) {
name
value
}
}
}
}
}with these variables:
{
"sourceId": "default",
"path": [
{
"type": "hosts"
}
],
"timerange": {
"from": 1539692147700,
"interval": "5m",
"to": 1539692747700
},
"filterQuery": null
}It works if you don't inline the metric or path arguments because then the previous condition short-circuits this.
There was a problem hiding this comment.
I'm going to move all the arguments from the sub branches (nodes and metric) to map. This will eliminate the need to parse the info object and fix this endpoint once and for all.
There was a problem hiding this comment.
That sounds like it would loose us most of the graphql advantages such as querying multiple node groups and metrics via aliasing. I think we should discuss this before throwing out the baby with the bathwater.
There was a problem hiding this comment.
The advantage is that it simplifies the code base. We don't have any plans to query the GraphQL endpoint with aliases at this point. If we need the ability to do that in the future for some use case then we can put the effort in at that time.
💔 Build Failed |
💚 Build Succeeded |
|
(pending green CI, of course 😉) |
💔 Build Failed |
|
jenkins test this |
💔 Build Failed |
💔 Build Failed |
This PR adds the log rate metric to the waffle map.