Browse Source

fix(ddm): enhance mri parsing (#59611)

Ogi 1 year ago
parent
commit
cd366b7309

+ 24 - 7
static/app/utils/metrics.spec.tsx

@@ -1,6 +1,11 @@
 import {formatMetricsUsingUnitAndOp, parseMRI} from 'sentry/utils/metrics';
 
 describe('parseMRI', () => {
+  it('should handle falsy values', () => {
+    expect(parseMRI('')).toEqual(null);
+    expect(parseMRI(undefined)).toEqual(null);
+  });
+
   it('should parse MRI with name, unit, and mri (custom use case)', () => {
     const mri = 'd:custom/sentry.events.symbolicator.query_task@second';
     const expectedResult = {
@@ -13,11 +18,11 @@ describe('parseMRI', () => {
   });
 
   it('should parse MRI with name, unit, and cleanMRI (transactions use case)', () => {
-    const mri = 'd:transactions/sentry.events.symbolicator.query_task@milisecond';
+    const mri = 'g:transactions/gauge@milisecond';
     const expectedResult = {
-      name: 'sentry.events.symbolicator.query_task',
+      name: 'gauge',
       unit: 'milisecond',
-      mri: 'd:transactions/sentry.events.symbolicator.query_task@milisecond',
+      mri: 'g:transactions/gauge@milisecond',
       useCase: 'transactions',
     };
     expect(parseMRI(mri)).toEqual(expectedResult);
@@ -35,16 +40,28 @@ describe('parseMRI', () => {
   });
 
   it('should extract MRI from nested operations', () => {
-    const mri = 'd:custom/sentry.events.symbolicator.query_task@second';
+    const mri = 'd:custom/foobar@none';
 
     const expectedResult = {
-      name: 'sentry.events.symbolicator.query_task',
-      unit: 'second',
-      mri: 'd:custom/sentry.events.symbolicator.query_task@second',
+      name: 'foobar',
+      unit: 'none',
+      mri: 'd:custom/foobar@none',
       useCase: 'custom',
     };
     expect(parseMRI(`sum(avg(${mri}))`)).toEqual(expectedResult);
   });
+
+  it('should extract MRI from nested operations (set)', () => {
+    const mri = 's:custom/foobar@none';
+
+    const expectedResult = {
+      name: 'foobar',
+      unit: 'none',
+      mri: 's:custom/foobar@none',
+      useCase: 'custom',
+    };
+    expect(parseMRI(`count_unique(${mri})`)).toEqual(expectedResult);
+  });
 });
 
 describe('formatMetricsUsingUnitAndOp', () => {

+ 11 - 7
static/app/utils/metrics.tsx

@@ -68,7 +68,7 @@ type MetricTag = {
 
 export function useMetricsTags(mri: string, projects: PageFilters['projects']) {
   const {slug} = useOrganization();
-  const {useCase} = parseMRI(mri);
+  const useCase = getUseCaseFromMRI(mri);
   return useApiQuery<MetricTag[]>(
     [
       `/organizations/${slug}/metrics/tags/`,
@@ -86,7 +86,7 @@ export function useMetricsTagValues(
   projects: PageFilters['projects']
 ) {
   const {slug} = useOrganization();
-  const {useCase} = parseMRI(mri);
+  const useCase = getUseCaseFromMRI(mri);
   return useApiQuery<MetricTag[]>(
     [
       `/organizations/${slug}/metrics/tags/${tag}/`,
@@ -136,7 +136,7 @@ export function useMetricsData({
   groupBy,
 }: MetricsQuery) {
   const {slug, features} = useOrganization();
-  const {useCase} = parseMRI(mri);
+  const useCase = getUseCaseFromMRI(mri);
   const field = op ? `${op}(${mri})` : mri;
 
   const interval = getMetricsInterval(datetime, mri);
@@ -248,7 +248,7 @@ function getMetricsInterval(dateTimeObj: DateTimeObject, mri: string) {
   }
 
   const diffInMinutes = getDiffInMinutes(dateTimeObj);
-  const {useCase} = parseMRI(mri);
+  const useCase = getUseCaseFromMRI(mri);
 
   if (diffInMinutes <= 60 && useCase === 'custom') {
     return '10s';
@@ -280,8 +280,12 @@ export function getReadableMetricType(type) {
 
 const noUnit = 'none';
 
-export function parseMRI(mri: string) {
-  const cleanMRI = mri.match(/d:[\w/.@]+/)?.[0] ?? mri;
+export function parseMRI(mri?: string) {
+  if (!mri) {
+    return null;
+  }
+
+  const cleanMRI = mri.match(/[cdegs]:[\w/.@]+/)?.[0] ?? mri;
 
   const name = cleanMRI.match(/^[a-z]:\w+\/(.+)(?:@\w+)$/)?.[1] ?? mri;
   const unit = cleanMRI.split('@').pop() ?? noUnit;
@@ -296,7 +300,7 @@ export function parseMRI(mri: string) {
   };
 }
 
-function getUseCaseFromMRI(mri?: string): UseCase {
+export function getUseCaseFromMRI(mri?: string): UseCase {
   if (mri?.includes('custom/')) {
     return 'custom';
   }

+ 2 - 2
static/app/views/ddm/queryBuilder.tsx

@@ -12,10 +12,10 @@ import {MetricsTag, SavedSearchType, TagCollection} from 'sentry/types';
 import {
   defaultMetricDisplayType,
   getReadableMetricType,
+  getUseCaseFromMRI,
   isAllowedOp,
   MetricDisplayType,
   MetricsQuery,
-  parseMRI,
   useMetricsMeta,
   useMetricsTags,
 } from 'sentry/utils/metrics';
@@ -195,7 +195,7 @@ function MetricSearchBar({tags, mri, disabled, onChange, query}: MetricSearchBar
   // TODO(ddm): try to use useApiQuery here
   const getTagValues = useCallback(
     async tag => {
-      const {useCase} = parseMRI(mri);
+      const useCase = getUseCaseFromMRI(mri);
       const tagsValues = await api.requestPromise(
         `/organizations/${org.slug}/metrics/tags/${tag.key}/`,
         {

+ 3 - 1
static/app/views/ddm/summaryTable.tsx

@@ -103,7 +103,9 @@ export function SummaryTable({
 
   const rows = series
     .map(s => {
-      const {name} = parseMRI(s.seriesName);
+      const parsed = parseMRI(s.seriesName);
+      const name = parsed?.name ?? s.seriesName;
+
       return {
         ...s,
         ...getValues(s.data),

+ 6 - 4
static/app/views/ddm/widget.tsx

@@ -250,7 +250,8 @@ function MetricWidgetBody({
 
 function getChartSeries(data: MetricsData, {focusedSeries, groupBy, hoveredLegend}) {
   // this assumes that all series have the same unit
-  const {unit} = parseMRI(Object.keys(data.groups[0]?.series ?? {})[0]);
+  const parsed = parseMRI(Object.keys(data.groups[0]?.series ?? {})[0]);
+  const unit = parsed?.unit ?? '';
 
   const series = data.groups.map(g => {
     return {
@@ -290,9 +291,10 @@ function getSeriesName(
   groupBy: MetricsQuery['groupBy']
 ) {
   if (isOnlyGroup && !groupBy?.length) {
-    const mri = Object.keys(group.series)?.[0] ?? '(none)';
-    const {name} = parseMRI(mri);
-    return name;
+    const mri = Object.keys(group.series)?.[0];
+    const parsed = parseMRI(mri);
+
+    return parsed?.name ?? '(none)';
   }
 
   return Object.entries(group.by)