From eb86aabf3edf960d0314e97ff2a9d7ce013b417a Mon Sep 17 00:00:00 2001 From: Amlan Kumar Nandy <45410599+amlannandy@users.noreply.github.com> Date: Mon, 12 May 2025 12:24:02 +0700 Subject: [PATCH] chore: improvements to the all attributes section in metric details (#7879) --- .../MetricDetails/AllAttributes.tsx | 179 +++++++++++++++--- .../MetricDetails/MetricDetails.styles.scss | 41 +++- .../MetricDetails/MetricDetails.tsx | 13 +- .../__tests__/AllAttributes.test.tsx | 102 +++++++++- .../MetricsExplorer/MetricDetails/types.ts | 8 + .../MetricsExplorer/MetricDetails/utils.tsx | 57 +++++- 6 files changed, 355 insertions(+), 45 deletions(-) diff --git a/frontend/src/container/MetricsExplorer/MetricDetails/AllAttributes.tsx b/frontend/src/container/MetricsExplorer/MetricDetails/AllAttributes.tsx index e46d626964..471151545d 100644 --- a/frontend/src/container/MetricsExplorer/MetricDetails/AllAttributes.tsx +++ b/frontend/src/container/MetricsExplorer/MetricDetails/AllAttributes.tsx @@ -1,19 +1,115 @@ -import { Button, Collapse, Input, Typography } from 'antd'; +import { Button, Collapse, Input, Menu, Popover, Typography } from 'antd'; import { ColumnsType } from 'antd/es/table'; import { ResizeTable } from 'components/ResizeTable'; import { DataType } from 'container/LogDetailedView/TableView'; -import { Search } from 'lucide-react'; +import { useNotifications } from 'hooks/useNotifications'; +import { Compass, Copy, Search } from 'lucide-react'; import { useCallback, useMemo, useState } from 'react'; +import { useCopyToClipboard } from 'react-use'; import { PANEL_TYPES } from '../../../constants/queryBuilder'; import ROUTES from '../../../constants/routes'; import { useHandleExplorerTabChange } from '../../../hooks/useHandleExplorerTabChange'; -import { AllAttributesProps } from './types'; +import { AllAttributesProps, AllAttributesValueProps } from './types'; import { getMetricDetailsQuery } from './utils'; +export function AllAttributesValue({ + filterKey, + filterValue, + goToMetricsExploreWithAppliedAttribute, +}: AllAttributesValueProps): JSX.Element { + const [visibleIndex, setVisibleIndex] = useState(5); + const [attributePopoverKey, setAttributePopoverKey] = useState( + null, + ); + const [, copyToClipboard] = useCopyToClipboard(); + const { notifications } = useNotifications(); + + const handleShowMore = (): void => { + setVisibleIndex(visibleIndex + 5); + }; + + const handleMenuItemClick = useCallback( + (key: string, attribute: string): void => { + switch (key) { + case 'open-in-explorer': + goToMetricsExploreWithAppliedAttribute(filterKey, attribute); + break; + case 'copy-attribute': + copyToClipboard(attribute); + notifications.success({ + message: 'Attribute copied!', + }); + break; + default: + break; + } + setAttributePopoverKey(null); + }, + [ + goToMetricsExploreWithAppliedAttribute, + filterKey, + copyToClipboard, + notifications, + ], + ); + + const attributePopoverContent = useCallback( + (attribute: string) => ( + , + label: 'Open in Explorer', + key: 'open-in-explorer', + }, + { + icon: , + label: 'Copy Attribute', + key: 'copy-attribute', + }, + ]} + onClick={(info): void => { + handleMenuItemClick(info.key, attribute); + }} + /> + ), + [handleMenuItemClick], + ); + return ( +
+ {filterValue.slice(0, visibleIndex).map((attribute) => ( + { + if (!open) { + setAttributePopoverKey(null); + } else { + setAttributePopoverKey(`${filterKey}-${attribute}`); + } + }} + > + + + ))} + {visibleIndex < filterValue.length && ( + + )} +
+ ); +} + function AllAttributes({ metricName, attributes, + metricType, }: AllAttributesProps): JSX.Element { const [searchString, setSearchString] = useState(''); const [activeKey, setActiveKey] = useState( @@ -22,9 +118,14 @@ function AllAttributes({ const { handleExplorerTabChange } = useHandleExplorerTabChange(); - const goToMetricsExploreWithAppliedAttribute = useCallback( - (key: string, value: string) => { - const compositeQuery = getMetricDetailsQuery(metricName, { key, value }); + const goToMetricsExplorerwithAppliedSpaceAggregation = useCallback( + (groupBy: string) => { + const compositeQuery = getMetricDetailsQuery( + metricName, + metricType, + undefined, + groupBy, + ); handleExplorerTabChange( PANEL_TYPES.TIME_SERIES, { @@ -35,13 +136,35 @@ function AllAttributes({ ROUTES.METRICS_EXPLORER_EXPLORER, ); }, - [metricName, handleExplorerTabChange], + [metricName, metricType, handleExplorerTabChange], + ); + const goToMetricsExploreWithAppliedAttribute = useCallback( + (key: string, value: string) => { + const compositeQuery = getMetricDetailsQuery(metricName, metricType, { + key, + value, + }); + handleExplorerTabChange( + PANEL_TYPES.TIME_SERIES, + { + query: compositeQuery, + name: metricName, + id: metricName, + }, + ROUTES.METRICS_EXPLORER_EXPLORER, + ); + }, + [metricName, metricType, handleExplorerTabChange], ); const filteredAttributes = useMemo( () => - attributes.filter((attribute) => - attribute.key.toLowerCase().includes(searchString.toLowerCase()), + attributes.filter( + (attribute) => + attribute.key.toLowerCase().includes(searchString.toLowerCase()) || + attribute.value.some((value) => + value.toLowerCase().includes(searchString.toLowerCase()), + ), ), [attributes, searchString], ); @@ -74,8 +197,17 @@ function AllAttributes({ className: 'metric-metadata-key', render: (field: { label: string; contribution: number }): JSX.Element => (
- {field.label} - {field.contribution} + + + {field.contribution} +
), }, @@ -88,23 +220,20 @@ function AllAttributes({ ellipsis: true, className: 'metric-metadata-value', render: (field: { key: string; value: string[] }): JSX.Element => ( -
- {field.value.map((attribute) => ( - - ))} -
+ ), }, ], - [goToMetricsExploreWithAppliedAttribute], + [ + goToMetricsExploreWithAppliedAttribute, + goToMetricsExplorerwithAppliedSpaceAggregation, + ], ); const items = useMemo( diff --git a/frontend/src/container/MetricsExplorer/MetricDetails/MetricDetails.styles.scss b/frontend/src/container/MetricsExplorer/MetricDetails/MetricDetails.styles.scss index 79811fd9d0..daf5c1b1b6 100644 --- a/frontend/src/container/MetricsExplorer/MetricDetails/MetricDetails.styles.scss +++ b/frontend/src/container/MetricsExplorer/MetricDetails/MetricDetails.styles.scss @@ -103,6 +103,21 @@ } .metrics-accordion { + .ant-table-body { + &::-webkit-scrollbar { + width: 2px; + } + + &::-webkit-scrollbar-track { + background: transparent; + } + + &::-webkit-scrollbar-thumb { + background: var(--bg-slate-300); + border-radius: 1px; + } + } + .metrics-accordion-header { display: flex; justify-content: space-between; @@ -142,11 +157,13 @@ .all-attributes-key { display: flex; justify-content: space-between; - .ant-typography:first-child { - font-family: 'Geist Mono'; - color: var(--bg-robin-400); + .ant-btn { + .ant-typography:first-child { + font-family: 'Geist Mono'; + color: var(--bg-robin-400); + } } - .ant-typography:last-child { + .all-attributes-contribution { font-family: 'Geist Mono'; color: var(--bg-vanilla-400); background-color: rgba(171, 189, 255, 0.1); @@ -166,8 +183,21 @@ display: flex; flex-direction: column; gap: 4px; - max-height: 120px; + max-height: 220px; overflow-y: scroll; + overflow-x: hidden; + &::-webkit-scrollbar { + width: 2px; + } + + &::-webkit-scrollbar-track { + background: transparent; + } + + &::-webkit-scrollbar-thumb { + background: var(--bg-slate-300); + border-radius: 1px; + } .ant-btn { text-align: left; width: fit-content; @@ -175,6 +205,7 @@ .ant-typography { color: var(--bg-vanilla-400); font-family: 'Geist Mono'; + text-overflow: ellipsis; } &:hover { background-color: var(--bg-slate-400); diff --git a/frontend/src/container/MetricsExplorer/MetricDetails/MetricDetails.tsx b/frontend/src/container/MetricsExplorer/MetricDetails/MetricDetails.tsx index e9c9da351d..97670b5061 100644 --- a/frontend/src/container/MetricsExplorer/MetricDetails/MetricDetails.tsx +++ b/frontend/src/container/MetricsExplorer/MetricDetails/MetricDetails.tsx @@ -82,7 +82,10 @@ function MetricDetails({ const goToMetricsExplorerwithSelectedMetric = useCallback(() => { if (metricName) { - const compositeQuery = getMetricDetailsQuery(metricName); + const compositeQuery = getMetricDetailsQuery( + metricName, + metric?.metadata?.metric_type, + ); handleExplorerTabChange( PANEL_TYPES.TIME_SERIES, { @@ -93,7 +96,7 @@ function MetricDetails({ ROUTES.METRICS_EXPLORER_EXPLORER, ); } - }, [metricName, handleExplorerTabChange]); + }, [metricName, handleExplorerTabChange, metric?.metadata?.metric_type]); const isMetricDetailsError = metricDetailsError || !metric; @@ -189,7 +192,11 @@ function MetricDetails({ refetchMetricDetails={refetchMetricDetails} /> {metric.attributes && ( - + )} )} diff --git a/frontend/src/container/MetricsExplorer/MetricDetails/__tests__/AllAttributes.test.tsx b/frontend/src/container/MetricsExplorer/MetricDetails/__tests__/AllAttributes.test.tsx index 02bd1e83d2..8f686e4ee4 100644 --- a/frontend/src/container/MetricsExplorer/MetricDetails/__tests__/AllAttributes.test.tsx +++ b/frontend/src/container/MetricsExplorer/MetricDetails/__tests__/AllAttributes.test.tsx @@ -1,9 +1,10 @@ import { fireEvent, render, screen } from '@testing-library/react'; +import { MetricType } from 'api/metricsExplorer/getMetricsList'; import * as useHandleExplorerTabChange from 'hooks/useHandleExplorerTabChange'; import { MetricDetailsAttribute } from '../../../../api/metricsExplorer/getMetricDetails'; import ROUTES from '../../../../constants/routes'; -import AllAttributes from '../AllAttributes'; +import AllAttributes, { AllAttributesValue } from '../AllAttributes'; jest.mock('react-router-dom', () => ({ ...jest.requireActual('react-router-dom'), @@ -19,6 +20,7 @@ jest }); const mockMetricName = 'test-metric'; +const mockMetricType = MetricType.GAUGE; const mockAttributes: MetricDetailsAttribute[] = [ { key: 'attribute1', @@ -35,7 +37,11 @@ const mockAttributes: MetricDetailsAttribute[] = [ describe('AllAttributes', () => { it('renders attributes section with title', () => { render( - , + , ); expect(screen.getByText('All Attributes')).toBeInTheDocument(); @@ -43,7 +49,11 @@ describe('AllAttributes', () => { it('renders all attribute keys and values', () => { render( - , + , ); // Check attribute keys are rendered @@ -58,7 +68,11 @@ describe('AllAttributes', () => { it('renders value counts correctly', () => { render( - , + , ); expect(screen.getByText('2')).toBeInTheDocument(); // For attribute1 @@ -66,17 +80,89 @@ describe('AllAttributes', () => { }); it('handles empty attributes array', () => { - render(); + render( + , + ); expect(screen.getByText('All Attributes')).toBeInTheDocument(); expect(screen.queryByText('No data')).toBeInTheDocument(); }); - it('clicking on an attribute value opens the explorer with the attribute filter applied', () => { + it('clicking on an attribute key opens the explorer with the attribute filter applied', () => { render( - , + , ); - fireEvent.click(screen.getByText('value1')); + fireEvent.click(screen.getByText('attribute1')); expect(mockHandleExplorerTabChange).toHaveBeenCalled(); }); + + it('filters attributes based on search input', () => { + render( + , + ); + fireEvent.change(screen.getByPlaceholderText('Search'), { + target: { value: 'value1' }, + }); + + expect(screen.getByText('attribute1')).toBeInTheDocument(); + expect(screen.getByText('value1')).toBeInTheDocument(); + }); +}); + +describe('AllAttributesValue', () => { + const mockGoToMetricsExploreWithAppliedAttribute = jest.fn(); + + it('renders all attribute values', () => { + render( + , + ); + expect(screen.getByText('value1')).toBeInTheDocument(); + expect(screen.getByText('value2')).toBeInTheDocument(); + }); + + it('loads more attributes when show more button is clicked', () => { + render( + , + ); + expect(screen.queryByText('value6')).not.toBeInTheDocument(); + fireEvent.click(screen.getByText('Show More')); + expect(screen.getByText('value6')).toBeInTheDocument(); + }); + + it('does not render show more button when there are no more attributes to show', () => { + render( + , + ); + expect(screen.queryByText('Show More')).not.toBeInTheDocument(); + }); }); diff --git a/frontend/src/container/MetricsExplorer/MetricDetails/types.ts b/frontend/src/container/MetricsExplorer/MetricDetails/types.ts index de4e864f0a..2453adf328 100644 --- a/frontend/src/container/MetricsExplorer/MetricDetails/types.ts +++ b/frontend/src/container/MetricsExplorer/MetricDetails/types.ts @@ -4,6 +4,7 @@ import { MetricDetailsAttribute, MetricDetailsDashboard, } from 'api/metricsExplorer/getMetricDetails'; +import { MetricType } from 'api/metricsExplorer/getMetricsList'; export interface MetricDetailsProps { onClose: () => void; @@ -27,6 +28,13 @@ export interface MetadataProps { export interface AllAttributesProps { attributes: MetricDetailsAttribute[]; metricName: string; + metricType: MetricType | undefined; +} + +export interface AllAttributesValueProps { + filterKey: string; + filterValue: string[]; + goToMetricsExploreWithAppliedAttribute: (key: string, value: string) => void; } export interface TopAttributesProps { diff --git a/frontend/src/container/MetricsExplorer/MetricDetails/utils.tsx b/frontend/src/container/MetricsExplorer/MetricDetails/utils.tsx index ec30207db2..c12130e284 100644 --- a/frontend/src/container/MetricsExplorer/MetricDetails/utils.tsx +++ b/frontend/src/container/MetricsExplorer/MetricDetails/utils.tsx @@ -60,8 +60,42 @@ export function determineIsMonotonic( export function getMetricDetailsQuery( metricName: string, + metricType: MetricType | undefined, filter?: { key: string; value: string }, + groupBy?: string, ): Query { + let timeAggregation; + let spaceAggregation; + let aggregateOperator; + switch (metricType) { + case MetricType.SUM: + timeAggregation = 'rate'; + spaceAggregation = 'sum'; + aggregateOperator = 'rate'; + break; + case MetricType.GAUGE: + timeAggregation = 'avg'; + spaceAggregation = 'avg'; + aggregateOperator = 'avg'; + break; + case MetricType.SUMMARY: + timeAggregation = 'noop'; + spaceAggregation = 'sum'; + aggregateOperator = 'noop'; + break; + case MetricType.HISTOGRAM: + case MetricType.EXPONENTIAL_HISTOGRAM: + timeAggregation = 'noop'; + spaceAggregation = 'p90'; + aggregateOperator = 'noop'; + break; + default: + timeAggregation = 'noop'; + spaceAggregation = 'noop'; + aggregateOperator = 'noop'; + break; + } + return { ...initialQueriesMap[DataSource.METRICS], builder: { @@ -70,11 +104,14 @@ export function getMetricDetailsQuery( ...initialQueriesMap[DataSource.METRICS].builder.queryData[0], aggregateAttribute: { key: metricName, - type: '', - id: `${metricName}----string--`, + type: metricType ?? '', + id: `${metricName}----${metricType}---string--`, + isColumn: true, + isJSON: false, }, - timeAggregation: 'rate', - spaceAggregation: 'sum', + aggregateOperator, + timeAggregation, + spaceAggregation, filters: { op: 'AND', items: filter @@ -91,6 +128,18 @@ export function getMetricDetailsQuery( ] : [], }, + groupBy: groupBy + ? [ + { + key: groupBy, + dataType: DataTypes.String, + type: 'tag', + isColumn: false, + isJSON: false, + id: `${groupBy}--string--tag--false`, + }, + ] + : [], }, ], queryFormulas: [],