Browse Source

ref(perf): Introduce `useSpanSamples` data hook (#67699)

Data hook that uses the `spans-samples` endpoint to fetch span samples
that fall into duration and time buckets.

There's already an existing `useSpanSamples` hook, but that one is not
appropriate for my use case:

- fetches its own bounds using `useSpanMetrics`
- uses `useApi` directly, which is an anti-pattern
- assembles the search query manually instead of using `MutableQuery`
- has no specS

This covers a lot of the same functionality. I'll merge them when I have
a chance.
George Gritsouk 11 months ago
parent
commit
0d466e624e

+ 144 - 0
static/app/views/performance/http/useSpanSamples.spec.tsx

@@ -0,0 +1,144 @@
+import type {ReactNode} from 'react';
+import {OrganizationFixture} from 'sentry-fixture/organization';
+
+import {makeTestQueryClient} from 'sentry-test/queryClient';
+import {reactHooks} from 'sentry-test/reactTestingLibrary';
+
+import {QueryClientProvider} from 'sentry/utils/queryClient';
+import {MutableSearch} from 'sentry/utils/tokenizeSearch';
+import useOrganization from 'sentry/utils/useOrganization';
+import usePageFilters from 'sentry/utils/usePageFilters';
+import {useSpanSamples} from 'sentry/views/performance/http/useSpanSamples';
+import type {IndexedProperty} from 'sentry/views/starfish/types';
+import {SpanIndexedField} from 'sentry/views/starfish/types';
+
+jest.mock('sentry/utils/usePageFilters');
+jest.mock('sentry/utils/useOrganization');
+
+function Wrapper({children}: {children?: ReactNode}) {
+  return (
+    <QueryClientProvider client={makeTestQueryClient()}>{children}</QueryClientProvider>
+  );
+}
+
+describe('useSpanSamples', () => {
+  const organization = OrganizationFixture();
+
+  jest.mocked(usePageFilters).mockReturnValue({
+    isReady: true,
+    desyncedFilters: new Set(),
+    pinnedFilters: new Set(),
+    shouldPersist: true,
+    selection: {
+      datetime: {
+        period: '10d',
+        start: null,
+        end: null,
+        utc: false,
+      },
+      environments: [],
+      projects: [],
+    },
+  });
+
+  jest.mocked(useOrganization).mockReturnValue(organization);
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('respects the `enabled` prop', () => {
+    const request = MockApiClient.addMockResponse({
+      url: `/api/0/organizations/${organization.slug}/spans-samples/`,
+      method: 'GET',
+      body: {data: []},
+    });
+
+    const {result} = reactHooks.renderHook(
+      ({fields, enabled}) => useSpanSamples({fields, enabled}),
+      {
+        wrapper: Wrapper,
+        initialProps: {
+          fields: [
+            SpanIndexedField.TRANSACTION_ID,
+            SpanIndexedField.ID,
+          ] as IndexedProperty[],
+          enabled: false,
+        },
+      }
+    );
+
+    expect(result.current.isFetching).toEqual(false);
+    expect(request).not.toHaveBeenCalled();
+  });
+
+  it('queries for current selection', async () => {
+    const request = MockApiClient.addMockResponse({
+      url: `/api/0/organizations/${organization.slug}/spans-samples/`,
+      method: 'GET',
+      body: {
+        data: [
+          {
+            'transaction.id': '7663aab8a',
+            'span.id': '3aab8a77fe231',
+          },
+        ],
+      },
+    });
+
+    const {result, waitFor} = reactHooks.renderHook(
+      ({filters, fields, referrer}) =>
+        useSpanSamples({
+          search: MutableSearch.fromQueryObject(filters),
+          fields,
+          referrer,
+          min: 100,
+          max: 900,
+        }),
+      {
+        wrapper: Wrapper,
+        initialProps: {
+          filters: {
+            'span.group': '221aa7ebd216',
+            release: '0.0.1',
+            environment: undefined,
+          },
+          fields: [
+            SpanIndexedField.TRANSACTION_ID,
+            SpanIndexedField.ID,
+          ] as IndexedProperty[],
+          referrer: 'api-spec',
+        },
+      }
+    );
+
+    expect(result.current.isLoading).toEqual(true);
+
+    expect(request).toHaveBeenCalledWith(
+      '/api/0/organizations/org-slug/spans-samples/',
+      expect.objectContaining({
+        method: 'GET',
+        query: {
+          additionalFields: ['transaction.id', 'span_id'],
+          project: [],
+          query: `span.group:221aa7ebd216 release:0.0.1`,
+          referrer: 'api-spec',
+          statsPeriod: '10d',
+          lowerBound: 100,
+          firstBound: 300,
+          secondBound: 600,
+          upperBound: 900,
+          utc: false,
+        },
+      })
+    );
+
+    await waitFor(() => expect(result.current.isLoading).toEqual(false));
+    expect(result.current.data).toEqual([
+      {
+        'transaction.id': '7663aab8a',
+        'span.id': '3aab8a77fe231',
+      },
+    ]);
+  });
+});

+ 95 - 0
static/app/views/performance/http/useSpanSamples.tsx

@@ -0,0 +1,95 @@
+// TODO: This is a _more general_ version of `useSpanSamples` from `/starfish/queries`. That hook should rely on this one _or_ they should be consolidated.
+
+import {defined} from 'sentry/utils';
+import {useApiQuery} from 'sentry/utils/queryClient';
+import type {MutableSearch} from 'sentry/utils/tokenizeSearch';
+import useOrganization from 'sentry/utils/useOrganization';
+import usePageFilters from 'sentry/utils/usePageFilters';
+import type {
+  IndexedProperty,
+  IndexedResponse,
+  SpanIndexedField,
+} from 'sentry/views/starfish/types';
+import {getDateConditions} from 'sentry/views/starfish/utils/getDateConditions';
+
+interface UseSpanSamplesOptions<Fields> {
+  enabled?: boolean;
+  fields?: Fields;
+  max?: number;
+  min?: number;
+  referrer?: string;
+  search?: MutableSearch;
+}
+
+export const useSpanSamples = <Fields extends IndexedProperty[]>(
+  options: UseSpanSamplesOptions<Fields> = {}
+) => {
+  const {
+    fields = [],
+    search = undefined,
+    referrer,
+    enabled,
+    min = undefined,
+    max = undefined,
+  } = options;
+
+  const {selection} = usePageFilters();
+
+  const organization = useOrganization();
+
+  if (defined(min) && min < 0) {
+    throw new Error('Minimum must be greater than 0');
+  }
+
+  if (defined(min) && defined(max) && min >= max) {
+    throw new Error('Maximum must be higher than minimum');
+  }
+
+  const dateConditions = getDateConditions(selection);
+
+  const result = useApiQuery<{
+    data:
+      | Pick<
+          IndexedResponse,
+          | Fields[number]
+          // These fields are returned by default
+          | SpanIndexedField.PROJECT
+          | SpanIndexedField.TRANSACTION_ID
+          | SpanIndexedField.TIMESTAMP
+          | SpanIndexedField.ID
+          | SpanIndexedField.PROFILE_ID
+        >[]
+      // This type is a little awkward but it explicitly states that the response could be empty. This doesn't enable unchecked access errors, but it at least indicates that it's possible that there's no data
+      // eslint-disable-next-line @typescript-eslint/ban-types
+      | [];
+    meta: 'hello';
+  }>(
+    [
+      `/api/0/organizations/${organization.slug}/spans-samples/`,
+      {
+        query: {
+          query: search?.formatString(),
+          project: selection.projects,
+          ...dateConditions,
+          ...{utc: selection.datetime.utc},
+          lowerBound: min,
+          firstBound: max && max * (1 / 3),
+          secondBound: max && max * (2 / 3),
+          upperBound: max,
+          additionalFields: fields,
+          referrer,
+        },
+      },
+    ],
+    {
+      enabled,
+      staleTime: Infinity,
+      retry: false,
+    }
+  );
+
+  return {
+    ...result,
+    data: result.data?.data ?? [],
+  };
+};

+ 7 - 1
static/app/views/starfish/types.tsx

@@ -142,7 +142,7 @@ export enum SpanIndexedField {
   RESPONSE_CODE = 'span.status_code',
 }
 
-export type SpanIndexedFieldTypes = {
+export type IndexedResponse = {
   [SpanIndexedField.SPAN_SELF_TIME]: number;
   [SpanIndexedField.SPAN_GROUP]: string;
   [SpanIndexedField.SPAN_MODULE]: string;
@@ -168,8 +168,14 @@ export type SpanIndexedFieldTypes = {
   [SpanIndexedField.INP_SCORE]: number;
   [SpanIndexedField.INP_SCORE_WEIGHT]: number;
   [SpanIndexedField.TOTAL_SCORE]: number;
+  [SpanIndexedField.RESPONSE_CODE]: string;
 };
 
+export type IndexedProperty = keyof IndexedResponse;
+
+// TODO: When convenient, remove this alias and use `IndexedResponse` everywhere
+export type SpanIndexedFieldTypes = IndexedResponse;
+
 export type Op = SpanIndexedFieldTypes[SpanIndexedField.SPAN_OP];
 
 export enum SpanFunction {