Fix incorrect metrics naming when group selected

Author: jotak

web/locales/en/plugin__netobserv-plugin.json

@@ -28,8 +28,6 @@
   "Scope": "Scope",
   "Long labels can reduce visibility.": "Long labels can reduce visibility.",
   "Truncate labels": "Truncate labels",
-  "Long labels can reduce visibility.": "Long labels can reduce visibility.",
-  "Truncate labels": "Truncate labels",
   "Display options": "Display options",
   "Source": "Source",
   "Destination": "Destination",
@@ -169,11 +167,11 @@
   "JSON": "JSON",
   "Kind not managed": "Kind not managed",
   "Unable to get flows": "Unable to get flows",
-  "Name": "Name",
   "Step into this {{name}}": "Step into this {{name}}",
   "Add {{name}} filter": "Add {{name}} filter",
   "Unpin this element": "Unpin this element",
   "Pin this element": "Pin this element",
+  "Name": "Name",
   "IP": "IP",
   "No information available for this content. Change scope to get more details.": "No information available for this content. Change scope to get more details.",
   "Source to destination:": "Source to destination:",

web/src/api/loki.ts

@@ -63,15 +63,21 @@ export interface RawTopologyMetrics {
   values: [number, unknown][];
 }
 
+export interface NameAndType {
+  name: string;
+  type: string;
+}
+
 export interface TopologyMetricPeer {
   id: string;
   addr?: string;
   namespace?: string;
-  owner?: { name: string; type: string };
-  resource?: { name: string; type: string };
+  owner?: NameAndType;
+  resource?: NameAndType;
   hostName?: string;
   resourceKind?: string;
-  displayName?: string;
+  isAmbiguous: boolean;
+  getDisplayName: (inclNamespace: boolean, disambiguate: boolean) => string | undefined;
 }
 
 export type TopologyMetrics = {

web/src/components/metrics/metrics-helper.tsx

@@ -5,7 +5,7 @@ import { NamedMetric, TopologyMetricPeer, TopologyMetrics } from '../../api/loki
 import { MetricScope, MetricType } from '../../model/flow-query';
 import { NodeData } from '../../model/topology';
 import { getDateFromUnix, getFormattedDate } from '../../utils/datetime';
-import { getFormattedRateValue, matchPeer } from '../../utils/metrics';
+import { getFormattedRateValue, isUnknownPeer, matchPeer } from '../../utils/metrics';
 import { TruncateLength } from '../dropdowns/truncate-dropdown';
 
 export type LegendDataItem = {
@@ -81,10 +81,13 @@ const getPeerName = (
   t: TFunction,
   peer: TopologyMetricPeer,
   scope: MetricScope,
-  truncateLength: TruncateLength = TruncateLength.OFF
+  truncateLength: TruncateLength,
+  inclNamespace: boolean,
+  disambiguate: boolean
 ): string => {
-  if (peer.displayName) {
-    return truncateParts(peer.displayName, truncateLength);
+  const name = peer.getDisplayName(inclNamespace, disambiguate);
+  if (name) {
+    return truncateParts(name, truncateLength);
   }
   if (scope === 'app') {
     // No peer distinction here
@@ -104,14 +107,16 @@ export const toNamedMetric = (
   t: TFunction,
   m: TopologyMetrics,
   truncateLength: TruncateLength,
+  inclNamespace: boolean,
+  disambiguate: boolean,
   data?: NodeData
 ): NamedMetric => {
-  const srcName = getPeerName(t, m.source, m.scope, truncateLength);
-  const srcFullName = getPeerName(t, m.source, m.scope);
-  const dstName = getPeerName(t, m.destination, m.scope, truncateLength);
-  const dstFullName = getPeerName(t, m.destination, m.scope);
+  const srcName = getPeerName(t, m.source, m.scope, truncateLength, inclNamespace, disambiguate);
+  const srcFullName = getPeerName(t, m.source, m.scope, TruncateLength.OFF, inclNamespace, disambiguate);
+  const dstName = getPeerName(t, m.destination, m.scope, truncateLength, inclNamespace, disambiguate);
+  const dstFullName = getPeerName(t, m.destination, m.scope, TruncateLength.OFF, inclNamespace, disambiguate);
   if (srcFullName === dstFullName) {
-    if (m.source.displayName) {
+    if (!isUnknownPeer(m.source)) {
       // E.g: namespace "netobserv" to "netobserv"
       return {
         ...m,

web/src/components/netflow-overview/netflow-overview.tsx

@@ -100,8 +100,8 @@ export const NetflowOverview: React.FC<NetflowOverviewProps> = ({
   //limit to top X since multiple queries can run in parallel
   const topKMetrics = metrics
     .sort((a, b) => getStat(b.stats, 'sum') - getStat(a.stats, 'sum'))
-    .map(m => toNamedMetric(t, m, truncateLength));
-  const namedTotalMetric = toNamedMetric(t, totalMetric, truncateLength);
+    .map(m => toNamedMetric(t, m, truncateLength, true, true));
+  const namedTotalMetric = toNamedMetric(t, totalMetric, truncateLength, false, false);
   const noInternalTopK = topKMetrics.filter(m => m.source.id !== m.destination.id);
 
   const smallerTexts = truncateLength >= TruncateLength.M;

web/src/components/netflow-topology/2d/topology-content.tsx

@@ -164,9 +164,11 @@ export const TopologyContent: React.FC<{
 
   const onFilter = React.useCallback(
     (data: Decorated<ElementData>) => {
-      const isFiltered = isElementFiltered(data, filters, t);
-      toggleElementFilter(data, isFiltered, filters, setFilters, t);
-      setSelectedIds([data.id]);
+      if (data.nodeType && data.peer) {
+        const isFiltered = isElementFiltered(data.nodeType, data.peer, 'any', filters, t);
+        toggleElementFilter(data.nodeType, data.peer, 'any', isFiltered, filters, setFilters, t);
+        setSelectedIds([data.id]);
+      }
     },
     [filters, setFilters, t]
   );

web/src/components/netflow-topology/element-field.tsx

@@ -11,23 +11,23 @@ export const ElementField: React.FC<{
   label: string;
   filterType: NodeType;
   forcedText?: string;
-  fields: Partial<TopologyMetricPeer>;
+  peer: TopologyMetricPeer;
   activeFilters: Filter[];
   setFilters: (filters: Filter[]) => void;
-}> = ({ id, label, filterType, forcedText, fields, activeFilters, setFilters }) => {
+}> = ({ id, label, filterType, forcedText, peer, activeFilters, setFilters }) => {
   return (
     <TextContent id={id} className="record-field-container">
       <Text component={TextVariants.h4}>{label}</Text>
       <Flex>
         <FlexItem flex={{ default: 'flex_1' }}>
-          {forcedText ? <Text>{forcedText}</Text> : <PeerResourceLink fields={fields} />}
+          {forcedText ? <Text>{forcedText}</Text> : <PeerResourceLink peer={peer} />}
         </FlexItem>
         <FlexItem>
           <SummaryFilterButton
             id={id + '-filter'}
             activeFilters={activeFilters}
             filterType={filterType}
-            fields={fields}
+            fields={peer}
             setFilters={setFilters}
           />
         </FlexItem>

web/src/components/netflow-topology/element-fields.tsx

@@ -4,6 +4,7 @@ import { useTranslation } from 'react-i18next';
 import { Filter } from '../../model/filters';
 import { NodeData } from '../../model/topology';
 import { ElementField } from './element-field';
+import { createPeer } from '../../utils/metrics';
 
 export const ElementFields: React.FC<{
   id: string;
@@ -26,7 +27,7 @@ export const ElementFields: React.FC<{
         forcedText={forceAsText ? data.peer.resource.name : undefined}
         activeFilters={activeFilters}
         filterType={'resource'}
-        fields={data.peer}
+        peer={data.peer}
         setFilters={setFilters}
       />
     );
@@ -41,7 +42,7 @@ export const ElementFields: React.FC<{
         forcedText={forceAsText ? data.peer.owner.name : undefined}
         activeFilters={activeFilters}
         filterType={'owner'}
-        fields={{ owner: data.peer.owner, namespace: data.peer.namespace }}
+        peer={createPeer({ owner: data.peer.owner, namespace: data.peer.namespace })}
         setFilters={setFilters}
       />
     );
@@ -56,7 +57,7 @@ export const ElementFields: React.FC<{
         forcedText={forceAsText ? data.peer.namespace : undefined}
         activeFilters={activeFilters}
         filterType={'namespace'}
-        fields={{ namespace: data.peer.namespace }}
+        peer={createPeer({ namespace: data.peer.namespace })}
         setFilters={setFilters}
       />
     );
@@ -71,7 +72,7 @@ export const ElementFields: React.FC<{
         forcedText={forceAsText ? data.peer.hostName : undefined}
         activeFilters={activeFilters}
         filterType={'host'}
-        fields={{ hostName: data.peer.hostName }}
+        peer={createPeer({ hostName: data.peer.hostName })}
         setFilters={setFilters}
       />
     );
@@ -85,7 +86,7 @@ export const ElementFields: React.FC<{
         label={t('IP')}
         activeFilters={activeFilters}
         filterType={'resource'}
-        fields={{ addr: data.peer.addr }}
+        peer={createPeer({ addr: data.peer.addr })}
         setFilters={setFilters}
       />
     );

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

@@ -166,14 +166,14 @@ export const ElementPanelMetricsContent: React.FC<{
   );
 
   if (element instanceof BaseNode && data) {
-    // TODO: fix metrics naming when a group is selected (ambiguous)
+    const leafData = element.getType() === 'group' ? undefined : data;
     const filteredMetrics = metrics.filter(m => m.source.id !== m.destination.id);
     const outMetrics = filteredMetrics
       .filter(m => matchPeer(data, m.source))
-      .map(m => toNamedMetric(t, m, truncateLength, data));
+      .map(m => toNamedMetric(t, m, truncateLength, false, false, leafData));
     const inMetrics = filteredMetrics
       .filter(m => matchPeer(data, m.destination))
-      .map(m => toNamedMetric(t, m, truncateLength, data));
+      .map(m => toNamedMetric(t, m, truncateLength, false, false, leafData));
     const outCount = outMetrics.reduce((prev, cur) => prev + getStat(cur.stats, metricFunction), 0);
     const inCount = inMetrics.reduce((prev, cur) => prev + getStat(cur.stats, metricFunction), 0);
     return (
@@ -198,10 +198,10 @@ export const ElementPanelMetricsContent: React.FC<{
     const bData = element.getTarget().getData();
     const aToBMetrics = metrics
       .filter(m => matchPeer(aData, m.source) && matchPeer(bData, m.destination))
-      .map(m => toNamedMetric(t, m, truncateLength));
+      .map(m => toNamedMetric(t, m, truncateLength, false, false));
     const bToAMetrics = metrics
       .filter(m => matchPeer(bData, m.source) && matchPeer(aData, m.destination))
-      .map(m => toNamedMetric(t, m, truncateLength));
+      .map(m => toNamedMetric(t, m, truncateLength, false, false));
     const aToBCount = aToBMetrics.reduce((prev, cur) => prev + getStat(cur.stats, metricFunction), 0);
     const bToACount = bToAMetrics.reduce((prev, cur) => prev + getStat(cur.stats, metricFunction), 0);
     return (
@@ -243,7 +243,7 @@ export const ElementPanel: React.FC<{
       return <Text component={TextVariants.h2}>{t('Edge')}</Text>;
     } else {
       const data = element.getData();
-      return <>{data && <PeerResourceLink fields={data.peer} />}</>;
+      return <>{data && <PeerResourceLink peer={data.peer} />}</>;
     }
   }, [element, t]);
 

web/src/components/netflow-topology/peer-resource-link.tsx

@@ -2,15 +2,14 @@ import * as React from 'react';
 import { ResourceLink } from '@openshift-console/dynamic-plugin-sdk';
 import { Text } from '@patternfly/react-core';
 import { TopologyMetricPeer } from '../../api/loki';
-import { peerNameAndKind } from '../../utils/metrics';
 
-export const PeerResourceLink: React.FC<{ fields: Partial<TopologyMetricPeer> }> = ({ fields }) => {
-  const nk = peerNameAndKind(fields, false);
-  if (nk) {
-    if (nk.kind) {
-      return <ResourceLink inline={true} kind={nk.kind} name={nk.name} namespace={fields.namespace} />;
+export const PeerResourceLink: React.FC<{ peer: TopologyMetricPeer }> = ({ peer }) => {
+  const name = peer.getDisplayName(false, false);
+  if (name) {
+    if (peer.resourceKind) {
+      return <ResourceLink inline={true} kind={peer.resourceKind} name={name} namespace={peer.namespace} />;
     } else {
-      return <Text>{nk.name}</Text>;
+      return <Text>{name}</Text>;
     }
   }
   return null;

web/src/model/topology.ts

@@ -22,7 +22,7 @@ import { K8sModel } from '@openshift-console/dynamic-plugin-sdk';
 import { getTopologyEdgeId } from '../utils/ids';
 import { MetricScopeOptions } from './metrics';
 import { MetricFunction, MetricScope, MetricType, NodeType } from './flow-query';
-import { createPeer, getFormattedValue, peerNameAndKind } from '../utils/metrics';
+import { createPeer, getFormattedValue } from '../utils/metrics';
 import { TruncateLength } from '../components/dropdowns/truncate-dropdown';
 
 export enum LayoutName {
@@ -211,10 +211,8 @@ const generateNode = (
   k8sModels: { [key: string]: K8sModel },
   isDark?: boolean
 ): NodeModel => {
-  const nk = peerNameAndKind(data.peer, false);
-  const label = nk?.name || (scope === 'host' ? t('External') : t('Unknown'))!;
-  data.peer.displayName = label;
-  const resourceKind = nk?.kind;
+  const label = data.peer.getDisplayName(false, false) || (scope === 'host' ? t('External') : t('Unknown'))!;
+  const resourceKind = data.peer.resourceKind;
   const secondaryLabel =
     data.nodeType !== 'namespace' &&
     ![
@@ -359,12 +357,13 @@ export const generateDataModel = (
   const opts = { ...DefaultOptions, ...options };
 
   const addGroup = (
-    metricFields: Omit<TopologyMetricPeer, 'id'>,
+    fields: Partial<TopologyMetricPeer>,
     scope: MetricScope,
     parent?: NodeModel,
     secondaryLabelPadding = false
   ): NodeModel => {
-    const groupDef = createPeer(metricFields);
+    const groupDef = createPeer(fields);
+    const groupName = groupDef.getDisplayName(false, false);
     let group = nodes.find(g => g.type === 'group' && g.id === groupDef.id);
     if (!group) {
       const data: NodeData = {
@@ -377,11 +376,11 @@ export const generateDataModel = (
         type: 'group',
         group: true,
         collapsed: options.startCollapsed,
-        label: groupDef.displayName,
+        label: groupName,
         style: { padding: secondaryLabelPadding ? 35 : 10 },
         data: {
           ...data,
-          name: groupDef.displayName,
+          name: groupName,
           isDark,
           labelPosition: LabelPosition.bottom,
           collapsible: true,
@@ -488,22 +487,27 @@ export const generateDataModel = (
     return ownerGroup || namespaceGroup || hostGroup;
   };
 
-  const dataBuilder: (p: TopologyMetricPeer) => NodeData =
-    metricScope === 'host'
-      ? p =>
-          _.isEmpty(p.hostName) ? { peer: p, nodeType: 'unknown' } : { peer: p, nodeType: 'host', canStepInto: true }
-      : metricScope === 'namespace'
-      ? p =>
-          _.isEmpty(p.namespace)
-            ? { peer: p, nodeType: 'unknown' }
-            : { peer: p, nodeType: 'namespace', canStepInto: true }
-      : metricScope === 'owner'
-      ? p => (p.owner ? { peer: p, nodeType: 'owner', canStepInto: true } : { peer: p, nodeType: 'unknown' })
-      : p => ({ peer: p, nodeType: 'resource' });
+  const peerToNodeData = (p: TopologyMetricPeer): NodeData => {
+    switch (metricScope) {
+      case 'host':
+        return _.isEmpty(p.hostName)
+          ? { peer: p, nodeType: 'unknown' }
+          : { peer: p, nodeType: 'host', canStepInto: true };
+      case 'namespace':
+        return _.isEmpty(p.namespace)
+          ? { peer: p, nodeType: 'unknown' }
+          : { peer: p, nodeType: 'namespace', canStepInto: true };
+      case 'owner':
+        return p.owner ? { peer: p, nodeType: 'owner', canStepInto: true } : { peer: p, nodeType: 'unknown' };
+      case 'resource':
+      default:
+        return { peer: p, nodeType: 'resource' };
+    }
+  };
 
   metrics.forEach(m => {
-    const srcNode = addNode(dataBuilder(m.source), metricScope);
-    const dstNode = addNode(dataBuilder(m.destination), metricScope);
+    const srcNode = addNode(peerToNodeData(m.source), metricScope);
+    const dstNode = addNode(peerToNodeData(m.destination), metricScope);
 
     if (options.edges && srcNode && dstNode && srcNode.id !== dstNode.id) {
       addEdge(