addressed feedback

pkg/loki/flow_query.go

@@ -270,16 +270,23 @@ func (q *FlowQueryBuilder) appendDNSFilter(sb *strings.Builder) {
 	sb.WriteString("|~`")
 	sb.WriteString(`"DnsId`)
 	sb.WriteString("`")
-	q.appendDNSLatencyFilter(sb)
-	q.appendDNSRCodeFilter(sb)
+	sb.WriteString("|~`")
+	sb.WriteString(`"DnsLatencyMs`)
+	sb.WriteString("`")
+	sb.WriteString("|~`")
+	sb.WriteString(`"DnsFlagsResponseCode"`)
+	sb.WriteString("`")
 }
 
 func (q *FlowQueryBuilder) appendDNSLatencyFilter(sb *strings.Builder) {
-	// ensure DnsLatencyMs field is specified
-	// |~`"DnsLatencyMs`
+	// ensure DnsLatencyMs field is specified and value is not zero
+	// |~`"DnsLatencyMs`!~`DnsLatencyMs%22:0[,}]`
 	sb.WriteString("|~`")
 	sb.WriteString(`"DnsLatencyMs`)
 	sb.WriteString("`")
+	sb.WriteString("!~`")
+	sb.WriteString(`"DnsLatencyMs":0[,}]`)
+	sb.WriteString("`")
 }
 
 func (q *FlowQueryBuilder) appendDNSRCodeFilter(sb *strings.Builder) {

web/src/api/routes.ts

@@ -1,7 +1,7 @@
 import axios from 'axios';
 import { Config, defaultConfig } from '../model/config';
 import { buildExportQuery } from '../model/export-query';
-import { FlowQuery, FlowScope, GenericAggregation } from '../model/flow-query';
+import { FlowQuery, FlowScope, GenericAggregation, isTimeMetric } from '../model/flow-query';
 import { ContextSingleton } from '../utils/context';
 import { TimeRange } from '../utils/datetime';
 import { parseTopologyMetrics, parseGenericMetrics } from '../utils/metrics';
@@ -81,6 +81,7 @@ export const getFlowMetrics = (params: FlowQuery, range: number | TimeRange): Pr
       range,
       params.aggregateBy as FlowScope,
       res.unixTimestamp,
+      !isTimeMetric(params.type),
       res.isMock
     );
   });
@@ -93,6 +94,7 @@ export const getFlowGenericMetrics = (params: FlowQuery, range: number | TimeRan
       range,
       params.aggregateBy as GenericAggregation,
       res.unixTimestamp,
+      !isTimeMetric(params.type),
       res.isMock
     );
   });

web/src/components/__tests-data__/metrics.ts

@@ -109,12 +109,14 @@ export const metrics = parseTopologyMetrics(
   [metric1, metric2, metric3],
   { from: 1653989800, to: 1653990100 },
   'resource',
-  0
+  0,
+  true
 ) as TopologyMetrics[];
 
 export const droppedMetrics = parseTopologyMetrics(
   [metric4],
   { from: 1653989800, to: 1653990100 },
   'resource',
-  0
+  0,
+  true
 ) as TopologyMetrics[];

web/src/components/dropdowns/metric-function-dropdown.tsx

@@ -1,7 +1,7 @@
 import { Dropdown, DropdownItem, DropdownPosition, DropdownToggle } from '@patternfly/react-core';
 import * as React from 'react';
 import { useTranslation } from 'react-i18next';
-import { MetricFunction, MetricType } from '../../model/flow-query';
+import { MetricFunction, MetricType, isTimeMetric } from '../../model/flow-query';
 
 export const TIME_METRIC_FUNCTIONS: MetricFunction[] = ['avg', 'min', 'max', 'p90', 'p99'];
 export const RATE_METRIC_FUNCTIONS: MetricFunction[] = ['last', 'avg', 'min', 'max', 'sum'];
@@ -27,7 +27,7 @@ export const MetricFunctionDropdown: React.FC<{
 
   const getMetricDisplay = React.useCallback(
     (mf: MetricFunction): string => {
-      const suffix = !['dnsLatencies', 'flowRtt'].includes(metricType || '') ? ' ' + t('rate') : '';
+      const suffix = !isTimeMetric(metricType) ? ' ' + t('rate') : '';
       switch (mf) {
         case 'sum':
           return t('Total');

web/src/components/netflow-topology/__tests-data__/metrics.ts

@@ -283,5 +283,6 @@ export const dataSample = parseTopologyMetrics(
   responseSample.data.result as RawTopologyMetrics[],
   { from: 1647965100, to: 1647965400 },
   'resource',
-  0
+  0,
+  true
 ) as TopologyMetrics[];

web/src/components/netflow-topology/element-panel-metrics.tsx

@@ -2,7 +2,7 @@ import { Flex, FlexItem, Radio, Text, TextVariants } from '@patternfly/react-cor
 import * as React from 'react';
 import { useTranslation } from 'react-i18next';
 import { TopologyMetrics } from '../../api/loki';
-import { MetricType } from '../../model/flow-query';
+import { MetricType, isTimeMetric } from '../../model/flow-query';
 import { getStat } from '../../model/metrics';
 import { decorated, NodeData } from '../../model/topology';
 import { matchPeer } from '../../utils/metrics';
@@ -24,7 +24,7 @@ export const ElementPanelMetrics: React.FC<{
   const { t } = useTranslation('plugin__netobserv-plugin');
   const [metricsRadio, setMetricsRadio] = React.useState<MetricsRadio>('both');
 
-  const useArea = !['dnsLatencies', 'flowRtt'].includes(metricType);
+  const useArea = !isTimeMetric(metricType);
   const titleStats = t('Stats');
 
   let id = '';
@@ -116,7 +116,7 @@ export const ElementPanelMetrics: React.FC<{
       <MetricsGraph
         id={id}
         metricType={metricType}
-        metricFunction={['dnsLatencies', 'flowRtt'].includes(metricType) ? 'sum' : 'avg'}
+        metricFunction={isTimeMetric(metricType) ? 'sum' : 'avg'}
         metrics={top5}
         limit={5}
         showArea={useArea}

web/src/components/netflow-topology/element-panel-stats.tsx

@@ -1,7 +1,7 @@
 import { Flex, FlexItem, Text, TextVariants } from '@patternfly/react-core';
 import * as React from 'react';
 import { useTranslation } from 'react-i18next';
-import { MetricType } from '../../model/flow-query';
+import { MetricType, isTimeMetric } from '../../model/flow-query';
 import { TopologyMetrics } from '../../api/loki';
 import { getStat } from '../../model/metrics';
 import { getFormattedValue } from '../../utils/metrics';
@@ -15,7 +15,7 @@ export const ElementPanelStats: React.FC<{
 }> = ({ metricsIn, metricsOut, metricsBoth, metricType, isEdge }) => {
   const { t } = useTranslation('plugin__netobserv-plugin');
 
-  const isTime = ['dnsLatencies', 'flowRtt'].includes(metricType);
+  const isTime = isTimeMetric(metricType);
   const latestIn = metricsIn.reduce((prev, cur) => prev + getStat(cur.stats, 'last'), 0);
   const averageIn = metricsIn.reduce((prev, cur) => prev + getStat(cur.stats, 'avg'), 0);
   const totalIn = metricsIn.reduce((prev, cur) => prev + getStat(cur.stats, 'sum'), 0);
@@ -25,8 +25,8 @@ export const ElementPanelStats: React.FC<{
   let latestBoth = metricsBoth.reduce((prev, cur) => prev + getStat(cur.stats, 'last'), 0);
   let averageBoth = metricsBoth.reduce((prev, cur) => prev + getStat(cur.stats, 'avg'), 0);
   if (isTime) {
-    latestBoth = latestBoth / 2;
-    averageBoth = averageBoth / 2;
+    latestBoth = latestIn && latestOut ? latestBoth / 2 : 0;
+    averageBoth = averageIn && averageOut ? averageBoth / 2 : 0;
   }
   const totalBoth = metricsBoth.reduce((prev, cur) => prev + getStat(cur.stats, 'sum'), 0);
   return (

web/src/components/netflow-traffic.tsx

@@ -58,6 +58,7 @@ import {
   filtersToString,
   FlowQuery,
   FlowScope,
+  isTimeMetric,
   Match,
   MetricFunction,
   MetricType,
@@ -319,7 +320,7 @@ export const NetflowTraffic: React.FC<{
 
   const updateTopologyMetricType = React.useCallback(
     (metricType: MetricType) => {
-      if (['dnsLatencies', 'flowRtt'].includes(metricType)) {
+      if (isTimeMetric(metricType)) {
         // fallback on average if current function not available for time queries
         if (!TIME_METRIC_FUNCTIONS.includes(topologyMetricFunction)) {
           setTopologyMetricFunction('avg');
@@ -790,7 +791,7 @@ export const NetflowTraffic: React.FC<{
         getMetrics(
           {
             ...fq,
-            function: ['dnsLatencies', 'flowRtt'].includes(topologyMetricType) ? topologyMetricFunction : undefined
+            function: isTimeMetric(topologyMetricType) ? topologyMetricFunction : undefined
           },
           range
         ).then(res => {

web/src/model/flow-query.ts

@@ -49,3 +49,7 @@ export const filtersToString = (filters: Filter[], matchAny: boolean): string =>
 export const filterByHashId = (hashId: string): string => {
   return encodeURIComponent(`_HashId="${hashId}"`);
 };
+
+export const isTimeMetric = (metricType: MetricType | undefined) => {
+  return ['dnsLatencies', 'flowRtt'].includes(metricType || '');
+};

web/src/utils/__tests__/back-and-forth.spec.ts

@@ -404,10 +404,10 @@ describe('Merge topology BNF', () => {
     expect(merged.metrics[2].source.namespace).toEqual('foo');
     expect(merged.metrics[2].destination.namespace).toBeUndefined();
     expect(merged.metrics[2].stats).toEqual({
-      avg: 2.63,
+      avg: 2.5,
       max: 5,
       sum: 50,
-      total: 710,
+      total: 712,
       latest: 0,
       min: 0,
       percentiles: [5, 5]
@@ -483,10 +483,10 @@ describe('Merge topology BNF', () => {
     expect(merged.metrics[2].source.namespace).toEqual('foo');
     expect(merged.metrics[2].destination.namespace).toBeUndefined();
     expect(merged.metrics[2].stats).toEqual({
-      avg: 2.63,
+      avg: 2.5,
       max: 5,
       sum: 50,
-      total: 710,
+      total: 712,
       latest: 0,
       min: 0,
       percentiles: [5, 5]

web/src/utils/__tests__/metrics.spec.ts

@@ -39,7 +39,7 @@ describe('normalize and computeStats', () => {
     ];
 
     const { start, end, step } = calibrateRange([values], { from: 1664372000, to: 1664372300 }, 1664372300, true);
-    const norm = normalizeMetrics(values, start, end, step);
+    const norm = normalizeMetrics(values, start, end, step, true);
     expect(norm).toEqual([
       [1664372000, 5],
       [1664372015, 5],
@@ -102,7 +102,7 @@ describe('normalize and computeStats', () => {
     ];
 
     const { start, end, step } = calibrateRange([values], 300, now, true);
-    const norm = normalizeMetrics(values, start, end, step);
+    const norm = normalizeMetrics(values, start, end, step, true);
     expect(norm).toEqual([
       [first, 5],
       [first + 15, 5],
@@ -155,7 +155,7 @@ describe('normalize and computeStats', () => {
     ];
 
     const { start, end, step } = calibrateRange([values], { from: 1664372000, to: 1664372300 }, 1664372300, true);
-    const norm = normalizeMetrics(values, start, end, step);
+    const norm = normalizeMetrics(values, start, end, step, true);
     expect(norm).toEqual([
       [1664372000, 5],
       [1664372015, 5],

web/src/utils/metrics.ts

@@ -36,6 +36,7 @@ export const parseTopologyMetrics = (
   range: number | TimeRange,
   aggregateBy: FlowScope,
   unixTimestamp: number,
+  forceZeros: boolean,
   isMock?: boolean
 ): TopologyMetrics[] => {
   const { start, end, step } = calibrateRange(
@@ -44,7 +45,7 @@ export const parseTopologyMetrics = (
     unixTimestamp,
     isMock
   );
-  const metrics = raw.map(r => parseTopologyMetric(r, start, end, step, aggregateBy));
+  const metrics = raw.map(r => parseTopologyMetric(r, start, end, step, aggregateBy, forceZeros));
 
   // Disambiguate display names with kind when necessary
   if (aggregateBy === 'owner' || aggregateBy === 'resource') {
@@ -93,6 +94,7 @@ export const parseGenericMetrics = (
   range: number | TimeRange,
   aggregateBy: GenericAggregation,
   unixTimestamp: number,
+  forceZeros: boolean,
   isMock?: boolean
 ): GenericMetric[] => {
   const { start, end, step } = calibrateRange(
@@ -101,7 +103,7 @@ export const parseGenericMetrics = (
     unixTimestamp,
     isMock
   );
-  return raw.map(r => parseGenericMetric(r, start, end, step, aggregateBy));
+  return raw.map(r => parseGenericMetric(r, start, end, step, aggregateBy, forceZeros));
 };
 
 export const createPeer = (fields: Partial<TopologyMetricPeer>): TopologyMetricPeer => {
@@ -153,9 +155,10 @@ const parseTopologyMetric = (
   start: number,
   end: number,
   step: number,
-  aggregateBy: FlowScope
+  aggregateBy: FlowScope,
+  forceZeros: boolean
 ): TopologyMetrics => {
-  const normalized = normalizeMetrics(raw.values, start, end, step);
+  const normalized = normalizeMetrics(raw.values, start, end, step, forceZeros);
   const stats = computeStats(normalized);
   const source = createPeer({
     addr: raw.metric.SrcAddr,
@@ -185,9 +188,10 @@ const parseGenericMetric = (
   start: number,
   end: number,
   step: number,
-  aggregateBy: GenericAggregation
+  aggregateBy: GenericAggregation,
+  forceZeros: boolean
 ): GenericMetric => {
-  const normalized = normalizeMetrics(raw.values, start, end, step);
+  const normalized = normalizeMetrics(raw.values, start, end, step, forceZeros);
   const stats = computeStats(normalized);
   const name =
     aggregateBy === 'droppedState'
@@ -262,22 +266,32 @@ export const normalizeMetrics = (
   start: number,
   end: number,
   step: number,
-  forceZeros?: boolean
+  forceZeros: boolean
 ): [number, number][] => {
-  // Normalize by counting all NaN as zeros
-  const normalized: [number, number][] = values.map(dp => {
-    let val = Number(dp[1]);
-    if (forceZeros && _.isNaN(val)) {
-      val = 0;
-    }
-    return [dp[0], val];
-  });
+  let normalized: [number, number][];
+  if (forceZeros) {
+    // Normalize by counting all NaN as zeros
+    normalized = values.map(dp => {
+      let val = Number(dp[1]);
+      if (_.isNaN(val)) {
+        val = 0;
+      }
+      return [dp[0], val];
+    });
 
-  // Normalize by filling missing datapoints with zeros
-  for (let current = start; current < end; current += step) {
-    if (!getValueCloseTo(normalized, current, step)) {
-      normalized.push([current, 0]);
+    // Normalize by filling missing datapoints with zeros
+    for (let current = start; current < end; current += step) {
+      if (!getValueCloseTo(normalized, current, step)) {
+        normalized.push([current, 0]);
+      }
     }
+  } else {
+    // skipping NaN
+    normalized = values
+      .filter(dp => !_.isNaN(Number(dp[1])))
+      .map(dp => {
+        return [dp[0], Number(dp[1])];
+      });
   }
 
   return normalized.sort((a, b) => a[0] - b[0]);
@@ -292,13 +306,13 @@ const getValueCloseTo = (values: [number, number][], timestamp: number, step: nu
 /**
  * computeStats computes avg, max and total. Input metric is always the bytes rate (Bps).
  */
-export const computeStats = (ts: [number, number][], skipZeros?: boolean): MetricStats => {
+export const computeStats = (ts: [number, number][]): MetricStats => {
   if (ts.length === 0) {
     return { sum: 0, latest: 0, avg: 0, min: 0, max: 0, percentiles: PERCENTILE_VALUES.map(() => 0), total: 0 };
   }
 
   const values = ts.map(dp => dp[1]);
-  const filteredValues = skipZeros ? values.filter(v => v !== 0) : values.filter(v => !Number.isNaN(v));
+  const filteredValues = values.filter(v => !Number.isNaN(v));
   if (!filteredValues.length) {
     return { sum: 0, latest: 0, avg: 0, min: 0, max: 0, percentiles: PERCENTILE_VALUES.map(() => 0), total: 0 };
   }
@@ -326,7 +340,11 @@ export const getFormattedValue = (v: number, mt: MetricType, mf: MetricFunction,
   if (mt === 'count' || mt === 'countDns') {
     return valueFormat(v);
   } else if (mt === 'dnsLatencies' || mt === 'flowRtt') {
-    return valueFormat(v, 2, t('ms'));
+    if (v) {
+      return valueFormat(v, 2, t('ms'));
+    } else {
+      return t('n/a');
+    }
   } else if (mf === 'sum') {
     switch (mt) {
       case 'droppedBytes':