Browse Source

feat(perf): Add HTTP module samples panel (#67344)

Just a basic panel for now. Actual contents coming in future PRs. Also,
this introduces some annoying duplication (the headers of these panels
are all the same, but we keep re-creating them) which I'll fix later.
George Gritsouk 11 months ago
parent
commit
b95af9eeaf

+ 18 - 1
static/app/views/performance/http/domainTransactionsTable.tsx

@@ -15,6 +15,7 @@ import {getFieldRenderer} from 'sentry/utils/discover/fieldRenderers';
 import {RATE_UNIT_TITLE, RateUnit, type Sort} from 'sentry/utils/discover/fields';
 import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
+import {TransactionCell} from 'sentry/views/performance/http/transactionCell';
 import {renderHeadCell} from 'sentry/views/starfish/components/tableCells/renderHeadCell';
 import type {MetricsResponse} from 'sentry/views/starfish/types';
 import {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
@@ -22,7 +23,9 @@ import {DataTitles} from 'sentry/views/starfish/views/spans/types';
 
 type Row = Pick<
   MetricsResponse,
+  | 'project.id'
   | 'transaction'
+  | 'transaction.method'
   | 'spm()'
   | 'http_response_rate(2)'
   | 'http_response_rate(4)'
@@ -101,6 +104,7 @@ interface Props {
   data: Row[];
   isLoading: boolean;
   sort: ValidSort;
+  domain?: string;
   error?: Error | null;
   meta?: EventsMetaType;
   pageLinks?: string;
@@ -113,6 +117,7 @@ export function DomainTransactionsTable({
   meta,
   pageLinks,
   sort,
+  domain,
 }: Props) {
   const location = useLocation();
   const organization = useOrganization();
@@ -147,7 +152,7 @@ export function DomainTransactionsTable({
               sortParameterName: QueryParameterNames.TRANSACTIONS_SORT,
             }),
           renderBodyCell: (column, row) =>
-            renderBodyCell(column, row, meta, location, organization),
+            renderBodyCell(column, row, meta, domain, location, organization),
         }}
         location={location}
       />
@@ -161,9 +166,21 @@ function renderBodyCell(
   column: Column,
   row: Row,
   meta: EventsMetaType | undefined,
+  domain: string | undefined,
   location: Location,
   organization: Organization
 ) {
+  if (column.key === 'transaction') {
+    return (
+      <TransactionCell
+        domain={domain}
+        project={String(row['project.id'])}
+        transaction={row.transaction}
+        transactionMethod={row['transaction.method']}
+      />
+    );
+  }
+
   if (!meta?.fields) {
     return row[column.key];
   }

+ 9 - 2
static/app/views/performance/http/httpDomainSummaryPage.spec.tsx

@@ -163,7 +163,6 @@ describe('HTTPSummaryPage', function () {
           dataset: 'spansMetrics',
           environment: [],
           field: [
-            'span.domain',
             'spm()',
             'avg(span.self_time)',
             'sum(span.self_time)',
@@ -190,7 +189,9 @@ describe('HTTPSummaryPage', function () {
           dataset: 'spansMetrics',
           environment: [],
           field: [
+            'project.id',
             'transaction',
+            'transaction.method',
             'spm()',
             'http_response_rate(2)',
             'http_response_rate(4)',
@@ -226,7 +227,9 @@ describe('HTTPSummaryPage', function () {
       body: {
         data: [
           {
+            'project.id': 8,
             transaction: '/api/users',
+            'transaction.method': 'GET',
             'spm()': 17.88,
             'http_response_rate(2)': 0.97,
             'http_response_rate(4)': 0.025,
@@ -264,7 +267,11 @@ describe('HTTPSummaryPage', function () {
     expect(screen.getByRole('columnheader', {name: 'Avg Duration'})).toBeInTheDocument();
     expect(screen.getByRole('columnheader', {name: 'Time Spent'})).toBeInTheDocument();
 
-    expect(screen.getByRole('cell', {name: '/api/users'})).toBeInTheDocument();
+    expect(screen.getByRole('cell', {name: 'GET /api/users'})).toBeInTheDocument();
+    expect(screen.getByRole('link', {name: 'GET /api/users'})).toHaveAttribute(
+      'href',
+      '/organizations/org-slug/performance/http/domains/?domain=%2A.sentry.dev&project=8&statsPeriod=10d&transaction=%2Fapi%2Fusers&transactionMethod=GET&transactionsCursor=0%3A20%3A0'
+    );
     expect(screen.getByRole('cell', {name: '17.9/s'})).toBeInTheDocument();
     expect(screen.getByRole('cell', {name: '97%'})).toBeInTheDocument();
     expect(screen.getByRole('cell', {name: '2.5%'})).toBeInTheDocument();

+ 6 - 1
static/app/views/performance/http/httpDomainSummaryPage.tsx

@@ -21,6 +21,7 @@ import {
   isAValidSort,
 } from 'sentry/views/performance/http/domainTransactionsTable';
 import {DurationChart} from 'sentry/views/performance/http/durationChart';
+import {HTTPSamplesPanel} from 'sentry/views/performance/http/httpSamplesPanel';
 import {ResponseRateChart} from 'sentry/views/performance/http/responseRateChart';
 import {ThroughputChart} from 'sentry/views/performance/http/throughputChart';
 import {MetricReadout} from 'sentry/views/performance/metricReadout';
@@ -60,7 +61,6 @@ export function HTTPDomainSummaryPage() {
   const {data: domainMetrics, isLoading: areDomainMetricsLoading} = useSpanMetrics({
     search: MutableSearch.fromQueryObject(filters),
     fields: [
-      SpanMetricsField.SPAN_DOMAIN,
       `${SpanFunction.SPM}()`,
       `avg(${SpanMetricsField.SPAN_SELF_TIME})`,
       `sum(${SpanMetricsField.SPAN_SELF_TIME})`,
@@ -114,7 +114,9 @@ export function HTTPDomainSummaryPage() {
   } = useSpanMetrics({
     search: MutableSearch.fromQueryObject(filters),
     fields: [
+      'project.id',
       'transaction',
+      'transaction.method',
       'spm()',
       'http_response_rate(2)',
       'http_response_rate(4)',
@@ -259,6 +261,7 @@ export function HTTPDomainSummaryPage() {
 
             <ModuleLayout.Full>
               <DomainTransactionsTable
+                domain={domain}
                 data={transactionsList}
                 error={transactionsListError}
                 isLoading={isTransactionsListLoading}
@@ -270,6 +273,8 @@ export function HTTPDomainSummaryPage() {
           </ModuleLayout.Layout>
         </Layout.Main>
       </Layout.Body>
+
+      <HTTPSamplesPanel />
     </React.Fragment>
   );
 }

+ 156 - 0
static/app/views/performance/http/httpSamplesPanel.spec.tsx

@@ -0,0 +1,156 @@
+import {OrganizationFixture} from 'sentry-fixture/organization';
+
+import {render, screen, waitForElementToBeRemoved} from 'sentry-test/reactTestingLibrary';
+
+import {useLocation} from 'sentry/utils/useLocation';
+import useOrganization from 'sentry/utils/useOrganization';
+import usePageFilters from 'sentry/utils/usePageFilters';
+import {HTTPSamplesPanel} from 'sentry/views/performance/http/httpSamplesPanel';
+
+jest.mock('sentry/utils/useLocation');
+jest.mock('sentry/utils/usePageFilters');
+jest.mock('sentry/utils/useOrganization');
+
+describe('HTTPSamplesPanel', function () {
+  const organization = OrganizationFixture();
+  let eventsRequestMock;
+
+  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(useLocation).mockReturnValue({
+    pathname: '',
+    search: '',
+    query: {
+      domain: '*.sentry.dev',
+      statsPeriod: '10d',
+      transaction: '/api/0/users',
+      transactionMethod: 'GET',
+    },
+    hash: '',
+    state: undefined,
+    action: 'PUSH',
+    key: '',
+  });
+
+  jest.mocked(useOrganization).mockReturnValue(organization);
+
+  beforeEach(function () {
+    eventsRequestMock = MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/events/`,
+      method: 'GET',
+      body: {
+        data: [],
+      },
+    });
+  });
+
+  afterAll(function () {
+    jest.resetAllMocks();
+  });
+
+  it('fetches panel data', async function () {
+    render(<HTTPSamplesPanel />);
+
+    expect(eventsRequestMock).toHaveBeenNthCalledWith(
+      1,
+      `/organizations/${organization.slug}/events/`,
+      expect.objectContaining({
+        method: 'GET',
+        query: {
+          dataset: 'spansMetrics',
+          environment: [],
+          field: [
+            'spm()',
+            'avg(span.self_time)',
+            'sum(span.self_time)',
+            'http_response_rate(3)',
+            'http_response_rate(4)',
+            'http_response_rate(5)',
+            'time_spent_percentage()',
+          ],
+          per_page: 50,
+          project: [],
+          query: 'span.module:http span.domain:"\\*.sentry.dev" transaction:/api/0/users',
+          referrer: 'api.starfish.http-module-samples-panel-metrics-ribbon',
+          statsPeriod: '10d',
+        },
+      })
+    );
+
+    await waitForElementToBeRemoved(() => screen.queryAllByTestId('loading-indicator'));
+  });
+
+  it('show basic transaction info', async function () {
+    MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/events/`,
+      method: 'GET',
+
+      match: [
+        MockApiClient.matchQuery({
+          referrer: 'api.starfish.http-module-samples-panel-metrics-ribbon',
+        }),
+      ],
+      body: {
+        data: [
+          {
+            'spm()': 22.18,
+            'http_response_rate(3)': 0.01,
+            'http_response_rate(4)': 0.025,
+            'http_response_rate(5)': 0.015,
+            'avg(span.self_time)': 140.2,
+            'sum(span.self_time)': 2709238,
+          },
+        ],
+        meta: {
+          fields: {
+            'spm()': 'rate',
+            'avg(span.self_time)': 'duration',
+            'http_response_rate(2)': 'percentage',
+            'http_response_rate(4)': 'percentage',
+            'http_response_rate(5)': 'percentage',
+            'sum(span.self_time)': 'duration',
+          },
+        },
+      },
+    });
+
+    render(<HTTPSamplesPanel />);
+
+    // Panel heading
+    expect(screen.getByRole('heading', {name: 'GET /api/0/users'})).toBeInTheDocument();
+
+    // Metrics ribbon
+    await waitForElementToBeRemoved(() => screen.queryAllByTestId('loading-indicator'));
+
+    expect(
+      screen.getByRole('heading', {name: 'Requests Per Minute'})
+    ).toBeInTheDocument();
+    expect(screen.getByRole('heading', {name: 'Avg Duration'})).toBeInTheDocument();
+    expect(screen.getByRole('heading', {name: '3XXs'})).toBeInTheDocument();
+    expect(screen.getByRole('heading', {name: '4XXs'})).toBeInTheDocument();
+    expect(screen.getByRole('heading', {name: '5XXs'})).toBeInTheDocument();
+    expect(screen.getByRole('heading', {name: 'Time Spent'})).toBeInTheDocument();
+
+    expect(screen.getByText('22.2/min')).toBeInTheDocument();
+    expect(screen.getByText('140.20ms')).toBeInTheDocument();
+    expect(screen.getByText('1%')).toBeInTheDocument();
+    expect(screen.getByText('2.5%')).toBeInTheDocument();
+    expect(screen.getByText('1.5%')).toBeInTheDocument();
+    expect(screen.getByText('45.15min')).toBeInTheDocument();
+  });
+});

+ 223 - 0
static/app/views/performance/http/httpSamplesPanel.tsx

@@ -0,0 +1,223 @@
+import {Link} from 'react-router';
+import styled from '@emotion/styled';
+import * as qs from 'query-string';
+
+import ProjectAvatar from 'sentry/components/avatar/projectAvatar';
+import {t} from 'sentry/locale';
+import {space} from 'sentry/styles/space';
+import {DurationUnit, RateUnit} from 'sentry/utils/discover/fields';
+import {PageAlertProvider} from 'sentry/utils/performance/contexts/pageAlert';
+import {decodeScalar} from 'sentry/utils/queryString';
+import {MutableSearch} from 'sentry/utils/tokenizeSearch';
+import {useLocation} from 'sentry/utils/useLocation';
+import useOrganization from 'sentry/utils/useOrganization';
+import useProjects from 'sentry/utils/useProjects';
+import useRouter from 'sentry/utils/useRouter';
+import {normalizeUrl} from 'sentry/utils/withDomainRequired';
+import {MetricReadout} from 'sentry/views/performance/metricReadout';
+import DetailPanel from 'sentry/views/starfish/components/detailPanel';
+import {getTimeSpentExplanation} from 'sentry/views/starfish/components/tableCells/timeSpentCell';
+import {useSpanMetrics} from 'sentry/views/starfish/queries/useSpanMetrics';
+import {
+  ModuleName,
+  SpanFunction,
+  SpanMetricsField,
+  type SpanMetricsQueryFilters,
+} from 'sentry/views/starfish/types';
+import {DataTitles, getThroughputTitle} from 'sentry/views/starfish/views/spans/types';
+
+type Query = {
+  domain?: string;
+  project?: string;
+  transaction?: string;
+  transactionMethod?: string;
+};
+
+export function HTTPSamplesPanel() {
+  const location = useLocation<Query>();
+  const query = location.query;
+
+  const router = useRouter();
+
+  const organization = useOrganization();
+
+  const projectId = decodeScalar(query.project);
+
+  const {projects} = useProjects();
+  const project = projects.find(p => projectId === p.id);
+
+  // `detailKey` controls whether the panel is open. If all required properties are available, concat them to make a key, otherwise set to `undefined` and hide the panel
+  const detailKey =
+    query.transaction && query.domain
+      ? [query.domain, query.transactionMethod, query.transaction]
+          .filter(Boolean)
+          .join(':')
+      : undefined;
+
+  const isPanelOpen = Boolean(detailKey);
+
+  const filters: SpanMetricsQueryFilters = {
+    'span.module': ModuleName.HTTP,
+    'span.domain': query.domain,
+    transaction: query.transaction,
+  };
+
+  const {
+    data: domainTransactionMetrics,
+    isFetching: areDomainTransactionMetricsFetching,
+  } = useSpanMetrics({
+    search: MutableSearch.fromQueryObject(filters),
+    fields: [
+      `${SpanFunction.SPM}()`,
+      `avg(${SpanMetricsField.SPAN_SELF_TIME})`,
+      `sum(${SpanMetricsField.SPAN_SELF_TIME})`,
+      'http_response_rate(3)',
+      'http_response_rate(4)',
+      'http_response_rate(5)',
+      `${SpanFunction.TIME_SPENT_PERCENTAGE}()`,
+    ],
+    enabled: isPanelOpen,
+    referrer: 'api.starfish.http-module-samples-panel-metrics-ribbon',
+  });
+
+  const handleClose = () => {
+    router.replace({
+      pathname: router.location.pathname,
+      query: {
+        ...router.location.query,
+        transaction: undefined,
+        transactionMethod: undefined,
+      },
+    });
+  };
+
+  return (
+    <PageAlertProvider>
+      <DetailPanel detailKey={detailKey} onClose={handleClose}>
+        <HeaderContainer>
+          {project && (
+            <SpanSummaryProjectAvatar
+              project={project}
+              direction="left"
+              size={40}
+              hasTooltip
+              tooltip={project.slug}
+            />
+          )}
+          <TitleContainer>
+            <Title>
+              <Link
+                to={normalizeUrl(
+                  `/organizations/${organization.slug}/performance/summary?${qs.stringify(
+                    {
+                      project: query.project,
+                      transaction: query.transaction,
+                    }
+                  )}`
+                )}
+              >
+                {query.transaction &&
+                query.transactionMethod &&
+                !query.transaction.startsWith(query.transactionMethod)
+                  ? `${query.transactionMethod} ${query.transaction}`
+                  : query.transaction}
+              </Link>
+            </Title>
+          </TitleContainer>
+        </HeaderContainer>
+
+        <MetricsRibbon>
+          <MetricReadout
+            align="left"
+            title={getThroughputTitle('http')}
+            value={domainTransactionMetrics?.[0]?.[`${SpanFunction.SPM}()`]}
+            unit={RateUnit.PER_MINUTE}
+            isLoading={areDomainTransactionMetricsFetching}
+          />
+
+          <MetricReadout
+            align="left"
+            title={DataTitles.avg}
+            value={
+              domainTransactionMetrics?.[0]?.[`avg(${SpanMetricsField.SPAN_SELF_TIME})`]
+            }
+            unit={DurationUnit.MILLISECOND}
+            isLoading={areDomainTransactionMetricsFetching}
+          />
+
+          <MetricReadout
+            align="left"
+            title={t('3XXs')}
+            value={domainTransactionMetrics?.[0]?.[`http_response_rate(3)`]}
+            unit="percentage"
+            isLoading={areDomainTransactionMetricsFetching}
+          />
+
+          <MetricReadout
+            align="left"
+            title={t('4XXs')}
+            value={domainTransactionMetrics?.[0]?.[`http_response_rate(4)`]}
+            unit="percentage"
+            isLoading={areDomainTransactionMetricsFetching}
+          />
+
+          <MetricReadout
+            align="left"
+            title={t('5XXs')}
+            value={domainTransactionMetrics?.[0]?.[`http_response_rate(5)`]}
+            unit="percentage"
+            isLoading={areDomainTransactionMetricsFetching}
+          />
+
+          <MetricReadout
+            align="left"
+            title={DataTitles.timeSpent}
+            value={domainTransactionMetrics?.[0]?.['sum(span.self_time)']}
+            unit={DurationUnit.MILLISECOND}
+            tooltip={getTimeSpentExplanation(
+              domainTransactionMetrics?.[0]?.['time_spent_percentage()'],
+              'db'
+            )}
+            isLoading={areDomainTransactionMetricsFetching}
+          />
+        </MetricsRibbon>
+      </DetailPanel>
+    </PageAlertProvider>
+  );
+}
+
+const SpanSummaryProjectAvatar = styled(ProjectAvatar)`
+  padding-right: ${space(1)};
+`;
+
+const HeaderContainer = styled('div')`
+  width: 100%;
+  padding-bottom: ${space(2)};
+  padding-top: ${space(1)};
+
+  display: grid;
+  grid-template-rows: auto auto auto;
+
+  @media (min-width: ${p => p.theme.breakpoints.small}) {
+    grid-template-rows: auto;
+    grid-template-columns: auto 1fr auto;
+  }
+`;
+
+const TitleContainer = styled('div')`
+  width: 100%;
+  position: relative;
+  height: 40px;
+`;
+
+const Title = styled('h4')`
+  position: absolute;
+  bottom: 0;
+  margin-bottom: 0;
+`;
+
+const MetricsRibbon = styled('div')`
+  display: flex;
+  flex-wrap: wrap;
+  gap: ${space(4)};
+`;

+ 54 - 0
static/app/views/performance/http/transactionCell.tsx

@@ -0,0 +1,54 @@
+import {Link} from 'react-router';
+import * as qs from 'query-string';
+
+import {useLocation} from 'sentry/utils/useLocation';
+import useOrganization from 'sentry/utils/useOrganization';
+import {normalizeUrl} from 'sentry/utils/withDomainRequired';
+import {OverflowEllipsisTextContainer} from 'sentry/views/starfish/components/textAlign';
+
+interface Props {
+  domain?: string;
+  project?: string;
+  transaction?: string;
+  transactionMethod?: string;
+}
+
+export function TransactionCell({
+  domain,
+  project,
+  transaction,
+  transactionMethod,
+}: Props) {
+  const location = useLocation();
+  const organization = useOrganization();
+
+  if (!domain || !transaction) {
+    return NULL_DESCRIPTION;
+  }
+
+  // TODO: This checks if the transaction name starts with the request method so we don't end up with labels like `GET GET /users` but any transaction name with an HTTP method prefix is incorrect, so it's not clear that we should cater to this
+  const label =
+    transactionMethod && !transaction.startsWith(transactionMethod)
+      ? `${transactionMethod} ${transaction}`
+      : transaction;
+
+  const pathname = normalizeUrl(
+    `/organizations/${organization.slug}/performance/http/domains/`
+  );
+
+  const query = {
+    ...location.query,
+    domain,
+    project,
+    transaction,
+    transactionMethod,
+  };
+
+  return (
+    <OverflowEllipsisTextContainer>
+      <Link to={`${pathname}?${qs.stringify(query)}`}>{label}</Link>
+    </OverflowEllipsisTextContainer>
+  );
+}
+
+const NULL_DESCRIPTION = <span>&lt;null&gt;</span>;