Browse Source

ref(tags): Update tag key-value request to use new dataset param (#74695)

This PR updates the fetch made to the
`organization/<slug>/tags/<key>/values/` endpoint on the frontend within
the issue stream search bar and the issue details page. Specifically, it
makes the request twice now, and utilizes the extra `dataset` query
parameter (which was added in [this
PR](https://github.com/getsentry/sentry/pull/74525)) to query only the
tag values from the Errors and IssuePlatform datasets. This is in an
effort to make the requests faster and more accurate.

Should be merged after #74696 so we can track the performance
improvements of these changes.
Michael Sun 7 months ago
parent
commit
2663e8f7d3

+ 6 - 0
static/app/actionCreators/tags.tsx

@@ -102,10 +102,12 @@ export function fetchTagValues({
   projectIds,
   search,
   sort,
+  dataset,
 }: {
   api: Client;
   orgSlug: string;
   tagKey: string;
+  dataset?: Dataset;
   endpointParams?: Query;
   includeReplays?: boolean;
   includeSessions?: boolean;
@@ -151,6 +153,10 @@ export function fetchTagValues({
     query.sort = sort;
   }
 
+  if (dataset) {
+    query.dataset = dataset;
+  }
+
   return api.requestPromise(url, {
     method: 'GET',
     query,

+ 23 - 8
static/app/views/issueDetails/groupEvents.tsx

@@ -18,6 +18,8 @@ import normalizeUrl from 'sentry/utils/url/normalizeUrl';
 import useApi from 'sentry/utils/useApi';
 import useCleanQueryParamsOnRouteLeave from 'sentry/utils/useCleanQueryParamsOnRouteLeave';
 import useOrganization from 'sentry/utils/useOrganization';
+import {Dataset} from 'sentry/views/alerts/rules/metric/types';
+import {mergeAndSortTagValues} from 'sentry/views/issueDetails/utils';
 import {makeGetIssueTagValues} from 'sentry/views/issueList/utils/getIssueTagValues';
 
 import AllEventsTable from './allEventsTable';
@@ -101,17 +103,30 @@ function UpdatedSearchBar({
   }, [data]);
 
   const tagValueLoader = useCallback(
-    (key: string, search: string) => {
+    async (key: string, search: string) => {
       const orgSlug = organization.slug;
       const projectIds = [group.project.id];
 
-      return fetchTagValues({
-        api,
-        orgSlug,
-        tagKey: key,
-        search,
-        projectIds,
-      });
+      const [eventsDatasetValues, issuePlatformDatasetValues] = await Promise.all([
+        fetchTagValues({
+          api,
+          orgSlug,
+          tagKey: key,
+          search,
+          projectIds,
+          dataset: Dataset.ERRORS,
+        }),
+        fetchTagValues({
+          api,
+          orgSlug,
+          tagKey: key,
+          search,
+          projectIds,
+          dataset: Dataset.ISSUE_PLATFORM,
+        }),
+      ]);
+
+      return mergeAndSortTagValues(eventsDatasetValues, issuePlatformDatasetValues);
     },
     [api, group.project.id, organization.slug]
   );

+ 34 - 1
static/app/views/issueDetails/utils.tsx

@@ -6,7 +6,7 @@ import {Client} from 'sentry/api';
 import {t} from 'sentry/locale';
 import ConfigStore from 'sentry/stores/configStore';
 import {useLegacyStore} from 'sentry/stores/useLegacyStore';
-import type {Group, GroupActivity} from 'sentry/types';
+import type {Group, GroupActivity, TagValue} from 'sentry/types';
 import type {Event} from 'sentry/types/event';
 import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
@@ -48,6 +48,39 @@ export function useDefaultIssueEvent() {
   const options = user ? user.options : null;
   return options?.defaultIssueEvent ?? 'recommended';
 }
+/**
+ * Combines two TagValue arrays and combines TagValue.count upon conflict
+ */
+export function mergeAndSortTagValues(
+  tagValues1: TagValue[],
+  tagValues2: TagValue[],
+  sort: 'count' | 'lastSeen' = 'lastSeen'
+): TagValue[] {
+  const tagValueCollection = tagValues1.reduce<Record<string, TagValue>>(
+    (acc, tagValue) => {
+      acc[tagValue.value] = tagValue;
+      return acc;
+    },
+    {}
+  );
+  tagValues2.forEach(tagValue => {
+    if (tagValueCollection[tagValue.value]) {
+      tagValueCollection[tagValue.value].count += tagValue.count;
+      if (tagValue.lastSeen > tagValueCollection[tagValue.value].lastSeen) {
+        tagValueCollection[tagValue.value].lastSeen = tagValue.lastSeen;
+      }
+    } else {
+      tagValueCollection[tagValue.value] = tagValue;
+    }
+  });
+  const allTagValues: TagValue[] = Object.values(tagValueCollection);
+  if (sort === 'count') {
+    allTagValues.sort((a, b) => b.count - a.count);
+  } else {
+    allTagValues.sort((a, b) => (b.lastSeen < a.lastSeen ? -1 : 1));
+  }
+  return allTagValues;
+}
 
 /**
  * Returns the environment name for an event or null

+ 62 - 3
static/app/views/issueList/searchBar.spec.tsx

@@ -1,9 +1,10 @@
 import {TagsFixture} from 'sentry-fixture/tags';
 
 import {initializeOrg} from 'sentry-test/initializeOrg';
-import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
+import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary';
 
 import TagStore from 'sentry/stores/tagStore';
+import type {Tag, TagValue} from 'sentry/types';
 import {IsFieldValues} from 'sentry/utils/fields';
 import IssueListSearchBar from 'sentry/views/issueList/searchBar';
 
@@ -205,8 +206,6 @@ describe('IssueListSearchBar', function () {
         body: [{key: 'someTag', name: 'Some Tag'}],
       });
 
-      defaultProps.organization.features = ['issue-stream-search-query-builder'];
-
       render(<IssueListSearchBar {...newDefaultProps} />, {
         router: routerWithFlag,
       });
@@ -238,4 +237,64 @@ describe('IssueListSearchBar', function () {
       expect(await screen.findByRole('option', {name: 'someTag'})).toBeInTheDocument();
     });
   });
+
+  describe('Tag Values', function () {
+    const {router: routerWithFlag, organization: orgWithFlag} = initializeOrg();
+    orgWithFlag.features = ['issue-stream-search-query-builder'];
+
+    const newDefaultProps = {
+      organization: orgWithFlag,
+      query: '',
+      statsPeriod: '7d',
+      onSearch: jest.fn(),
+    };
+
+    it('displays the correct tag values for a key', async () => {
+      const tagKey = 'random';
+      const tagValue = 'randomValue';
+      const tagValueResponse: TagValue[] = [
+        {
+          name: tagValue,
+          value: tagValue,
+          count: 1,
+          firstSeen: '2021-01-01T00:00:00Z',
+          lastSeen: '2021-01-01T00:00:00Z',
+          email: 'a@sentry.io',
+          username: 'a',
+          id: '1',
+          ip_address: '1',
+        },
+      ];
+      const tag: Tag = {
+        key: tagKey,
+        name: tagKey,
+      };
+
+      MockApiClient.addMockResponse({
+        url: '/organizations/org-slug/tags/',
+        method: 'GET',
+        body: [tag],
+      });
+      const tagValueMock = MockApiClient.addMockResponse({
+        url: `/organizations/org-slug/tags/${tagKey}/values/`,
+        method: 'GET',
+        body: tagValueResponse,
+      });
+
+      render(<IssueListSearchBar {...newDefaultProps} />, {
+        router: routerWithFlag,
+      });
+
+      await userEvent.click(screen.getByRole('combobox', {name: 'Add a search term'}));
+      await userEvent.paste(tagKey, {delay: null});
+      await userEvent.click(screen.getByRole('option', {name: tagKey}));
+      expect(await screen.findByRole('option', {name: tagValue})).toBeInTheDocument();
+
+      await waitFor(() => {
+        // Expected twice since we make one request for values in events dataset
+        // and another for values in IssuePlatform dataset.
+        expect(tagValueMock).toHaveBeenCalledTimes(2);
+      });
+    });
+  });
 });

+ 25 - 9
static/app/views/issueList/searchBar.tsx

@@ -24,6 +24,8 @@ import useApi from 'sentry/utils/useApi';
 import usePageFilters from 'sentry/utils/usePageFilters';
 import type {WithIssueTagsProps} from 'sentry/utils/withIssueTags';
 import withIssueTags from 'sentry/utils/withIssueTags';
+import {Dataset} from 'sentry/views/alerts/rules/metric/types';
+import {mergeAndSortTagValues} from 'sentry/views/issueDetails/utils';
 import {makeGetIssueTagValues} from 'sentry/views/issueList/utils/getIssueTagValues';
 import {useFetchIssueTags} from 'sentry/views/issueList/utils/useFetchIssueTags';
 
@@ -109,7 +111,7 @@ function IssueListSearchBar({organization, tags, onClose, ...props}: Props) {
   });
 
   const tagValueLoader = useCallback(
-    (key: string, search: string) => {
+    async (key: string, search: string) => {
       const orgSlug = organization.slug;
       const projectIds = pageFilters.projects.map(id => id.toString());
       const endpointParams = {
@@ -122,14 +124,28 @@ function IssueListSearchBar({organization, tags, onClose, ...props}: Props) {
         statsPeriod: pageFilters.datetime.period,
       };
 
-      return fetchTagValues({
-        api,
-        orgSlug,
-        tagKey: key,
-        search,
-        projectIds,
-        endpointParams,
-      });
+      const [eventsDatasetValues, issuePlatformDatasetValues] = await Promise.all([
+        fetchTagValues({
+          api,
+          orgSlug,
+          tagKey: key,
+          search,
+          projectIds,
+          endpointParams,
+          dataset: Dataset.ERRORS,
+        }),
+        fetchTagValues({
+          api,
+          orgSlug,
+          tagKey: key,
+          search,
+          projectIds,
+          endpointParams,
+          dataset: Dataset.ISSUE_PLATFORM,
+        }),
+      ]);
+
+      return mergeAndSortTagValues(eventsDatasetValues, issuePlatformDatasetValues);
     },
     [
       api,

+ 138 - 0
static/app/views/issueList/utils.spec.tsx

@@ -1,3 +1,6 @@
+import type {TagValue} from 'sentry/types';
+import {mergeAndSortTagValues} from 'sentry/views/issueDetails/utils';
+
 import {getTabs} from './utils';
 
 describe('getTabs', () => {
@@ -17,4 +20,139 @@ describe('getTabs', () => {
       ['is:reprocessing', expect.objectContaining({name: 'Reprocessing'})],
     ]);
   });
+
+  it('merges and sorts tagValues by count correctly', () => {
+    const defaultTagValueFields = {
+      email: '',
+      id: '',
+      name: '',
+      username: '',
+      ip_address: '',
+    };
+    const tagValues1: TagValue[] = [
+      {
+        value: 'a',
+        count: 1,
+        lastSeen: '2021-01-01T00:00:00',
+        firstSeen: '2021-01-01T00:00:00',
+        ...defaultTagValueFields,
+      },
+      {
+        value: 'b',
+        count: 1,
+        lastSeen: '2021-01-02T00:00:00',
+        firstSeen: '2021-01-02T00:00:00',
+        ...defaultTagValueFields,
+      },
+    ];
+
+    const tagValues2: TagValue[] = [
+      {
+        value: 'a',
+        count: 1,
+        lastSeen: '2021-01-01T00:00:00',
+        firstSeen: '2021-01-01T00:00:00',
+        ...defaultTagValueFields,
+      },
+      {
+        value: 'c',
+        count: 3,
+        lastSeen: '2021-01-03T00:00:00',
+        firstSeen: '2021-01-03T00:00:00',
+        ...defaultTagValueFields,
+      },
+    ];
+    const sortByCount = mergeAndSortTagValues(tagValues1, tagValues2, 'count');
+    expect(sortByCount).toEqual([
+      {
+        value: 'c',
+        count: 3,
+        lastSeen: '2021-01-03T00:00:00',
+        firstSeen: '2021-01-03T00:00:00',
+        ...defaultTagValueFields,
+      },
+      {
+        value: 'a',
+        count: 2,
+        lastSeen: '2021-01-01T00:00:00',
+        firstSeen: '2021-01-01T00:00:00',
+        ...defaultTagValueFields,
+      },
+      {
+        value: 'b',
+        count: 1,
+        lastSeen: '2021-01-02T00:00:00',
+        firstSeen: '2021-01-02T00:00:00',
+        ...defaultTagValueFields,
+      },
+    ]);
+  });
+
+  it('merges and sorts tagValues by lastSeen correctly', () => {
+    const defaultTagValueFields = {
+      email: '',
+      id: '',
+      name: '',
+      username: '',
+      ip_address: '',
+    };
+    const tagValues1: TagValue[] = [
+      {
+        value: 'a',
+        count: 1,
+        lastSeen: '2021-01-01T00:00:00',
+        firstSeen: '2021-01-01T00:00:00',
+        ...defaultTagValueFields,
+      },
+      {
+        value: 'b',
+        count: 1,
+        lastSeen: '2021-01-02T00:00:00',
+        firstSeen: '2021-01-02T00:00:00',
+        ...defaultTagValueFields,
+      },
+    ];
+
+    const tagValues2: TagValue[] = [
+      {
+        value: 'a',
+        count: 1,
+        lastSeen: '2021-01-01T00:00:00',
+        firstSeen: '2021-01-01T00:00:00',
+        ...defaultTagValueFields,
+      },
+      {
+        value: 'c',
+        count: 3,
+        lastSeen: '2021-01-03T00:00:00',
+        firstSeen: '2021-01-03T00:00:00',
+        ...defaultTagValueFields,
+      },
+    ];
+
+    const sortByLastSeen = mergeAndSortTagValues(tagValues1, tagValues2, 'lastSeen');
+    expect(sortByLastSeen).toEqual([
+      {
+        value: 'c',
+        count: 3,
+        lastSeen: '2021-01-03T00:00:00',
+        firstSeen: '2021-01-03T00:00:00',
+        ...defaultTagValueFields,
+      },
+      {
+        value: 'b',
+        count: 1,
+        lastSeen: '2021-01-02T00:00:00',
+        firstSeen: '2021-01-02T00:00:00',
+        ...defaultTagValueFields,
+      },
+      {
+        value: 'a',
+        count: 2,
+        lastSeen: '2021-01-01T00:00:00',
+        firstSeen: '2021-01-01T00:00:00',
+        ...defaultTagValueFields,
+      },
+    ]);
+  });
 });