Browse Source

fix(metrics): Duplicate metrics name detection performance (#72106)

Detect duplicate metric names in `O(n)` instead of `O(n^2)`.

Part of https://github.com/getsentry/sentry/issues/71979
ArthurKnaus 9 months ago
parent
commit
c20d165f14

+ 58 - 0
static/app/views/metrics/queryBuilder.spec.tsx

@@ -0,0 +1,58 @@
+import type {MetricMeta} from 'sentry/types/metrics';
+import {getMetricsWithDuplicateNames} from 'sentry/views/metrics/queryBuilder';
+
+function createMetricMeta(name: string, unit: string): MetricMeta {
+  return {
+    mri: `d:custom/${name}@${unit}`,
+    blockingStatus: [],
+    operations: [],
+    projectIds: [],
+    type: 'd',
+    unit: unit,
+  };
+}
+
+describe('getMetricsWithDuplicateNames', () => {
+  it('should return a duplicate metric', () => {
+    const metrics: MetricMeta[] = [
+      createMetricMeta('metric1', 'none'),
+      createMetricMeta('metric1', 'seconds'),
+      createMetricMeta('metric2', 'milliseconds'),
+    ];
+    const result = getMetricsWithDuplicateNames(metrics);
+    expect(result).toEqual(
+      new Set(['d:custom/metric1@none', 'd:custom/metric1@seconds'])
+    );
+  });
+
+  it('should multiple duplicate metrics', () => {
+    const metrics: MetricMeta[] = [
+      createMetricMeta('metric1', 'none'),
+      createMetricMeta('metric1', 'seconds'),
+
+      createMetricMeta('metric2', 'none'),
+      createMetricMeta('metric2', 'milliseconds'),
+
+      createMetricMeta('metric3', 'none'),
+    ];
+    const result = getMetricsWithDuplicateNames(metrics);
+    expect(result).toEqual(
+      new Set([
+        'd:custom/metric1@none',
+        'd:custom/metric1@seconds',
+        'd:custom/metric2@none',
+        'd:custom/metric2@milliseconds',
+      ])
+    );
+  });
+
+  it('should return an empty set if there are no duplicates', () => {
+    const metrics: MetricMeta[] = [
+      createMetricMeta('metric1', 'none'),
+      createMetricMeta('metric2', 'seconds'),
+      createMetricMeta('metric3', 'milliseconds'),
+    ];
+    const result = getMetricsWithDuplicateNames(metrics);
+    expect(result).toEqual(new Set());
+  });
+});

+ 47 - 6
static/app/views/metrics/queryBuilder.tsx

@@ -68,6 +68,49 @@ function useMriMode() {
   return mriMode;
 }
 
+/**
+ * Returns a set of MRIs that have duplicate names but different units
+ */
+export function getMetricsWithDuplicateNames(metrics: MetricMeta[]): Set<MRI> {
+  const metricNameMap = new Map<string, MRI[]>();
+  const duplicateNames: string[] = [];
+
+  for (const metric of metrics) {
+    const metricName = parseMRI(metric.mri)?.name;
+    if (!metricName) {
+      continue;
+    }
+
+    if (metricNameMap.has(metricName)) {
+      const mapEntry = metricNameMap.get(metricName);
+      mapEntry?.push(metric.mri);
+      duplicateNames.push(metricName);
+    } else {
+      metricNameMap.set(metricName, [metric.mri]);
+    }
+  }
+
+  const duplicateMetrics = new Set<MRI>();
+  for (const name of duplicateNames) {
+    const duplicates = metricNameMap.get(name);
+    if (!duplicates) {
+      continue;
+    }
+    duplicates.forEach(duplicate => duplicateMetrics.add(duplicate));
+  }
+
+  return duplicateMetrics;
+}
+
+/**
+ * Returns a set of MRIs that have duplicate names but different units
+ */
+function useMetricsWithDuplicateNames(metrics: MetricMeta[]): Set<MRI> {
+  return useMemo(() => {
+    return getMetricsWithDuplicateNames(metrics);
+  }, [metrics]);
+}
+
 export const QueryBuilder = memo(function QueryBuilder({
   metricsQuery,
   projects: projectIds,
@@ -93,6 +136,8 @@ export const QueryBuilder = memo(function QueryBuilder({
     }
   );
 
+  const metricsWithDuplicateNames = useMetricsWithDuplicateNames(meta);
+
   const selectedProjects = useMemo(
     () =>
       projects.filter(project =>
@@ -228,11 +273,7 @@ export const QueryBuilder = memo(function QueryBuilder({
   const mriOptions = useMemo(
     () =>
       displayedMetrics.map<ComboBoxOption<MRI>>(metric => {
-        const isDuplicateWithDifferentUnit = displayedMetrics.some(
-          displayedMetric =>
-            metric.mri !== displayedMetric.mri &&
-            parseMRI(metric.mri)?.name === parseMRI(displayedMetric.mri)?.name
-        );
+        const isDuplicateWithDifferentUnit = metricsWithDuplicateNames.has(metric.mri);
         const trailingItems: React.ReactNode[] = [];
         if (isDuplicateWithDifferentUnit) {
           trailingItems.push(<IconWarning key="warning" size="xs" color="yellow400" />);
@@ -267,7 +308,7 @@ export const QueryBuilder = memo(function QueryBuilder({
           trailingItems,
         };
       }),
-    [displayedMetrics, mriMode, onChange, selectedProjects]
+    [displayedMetrics, metricsWithDuplicateNames, mriMode, onChange, selectedProjects]
   );
 
   const projectIdStrings = useMemo(() => projectIds.map(String), [projectIds]);