Browse Source

fix(database): Improve missing data message (#61868)

Several important improvements to the "No queries found" banner in the
Database overview.

1. The "No queries found" message _only shows up if there's no data on
the screen_. Previously, it did it's own check. This was portable, but
it was inconsistent. People were seeing the banner even though there's
data below. Now, the banner component is told by the parent whether
there's data or not
2. Small improvements to copy and UI. Prevents duplicate projects in the
lists, extracts some components, improves grammar, removes unused code.
3. New "You have omitted projects" message for those who have been
denylisted, so they can contact support. Checks the metrics ingestion
flag, since that's a 1:1 proxy for our denylisting.
4. The "No queries found", "You have outdated SDKs" and "Your projects
were omitted" messages are now more independent. The omission message
_always_ show up if true, even if there's data! The SDK message only
shows up if there's no data. The "No data" messages shows up if there's
no data

---------

Co-authored-by: Abhijeet Prasad <aprasad@sentry.io>
George Gritsouk 1 year ago
parent
commit
b1ae27e095

+ 16 - 1
static/app/views/performance/database/databaseLandingPage.tsx

@@ -107,6 +107,16 @@ export function DatabaseLandingPage() {
     'api.starfish.span-landing-page-metrics-chart'
   );
 
+  const isCriticalDataLoading =
+    isThroughputDataLoading || isDurationDataLoading || queryListResponse.isLoading;
+
+  const isAnyCriticalDataAvailable =
+    (queryListResponse.data ?? []).length > 0 ||
+    durationData[`${selectedAggregate}(span.self_time)`].data?.some(
+      ({value}) => value > 0
+    ) ||
+    throughputData['spm()'].data?.some(({value}) => value > 0);
+
   useSynchronizeCharts([!isThroughputDataLoading && !isDurationDataLoading]);
 
   return (
@@ -132,7 +142,12 @@ export function DatabaseLandingPage() {
 
       <Layout.Body>
         <Layout.Main fullWidth>
-          {!onboardingProject && <NoDataMessage Wrapper={AlertBanner} />}
+          {!onboardingProject && !isCriticalDataLoading && (
+            <NoDataMessage
+              Wrapper={AlertBanner}
+              isDataAvailable={isAnyCriticalDataAvailable}
+            />
+          )}
           <FloatingFeedbackWidget />
           <PaddedContainer>
             <PageFilterBar condensed>

+ 44 - 57
static/app/views/performance/database/noDataMessage.spec.tsx

@@ -20,7 +20,13 @@ describe('NoDataMessage', () => {
     MockApiClient.clearMockResponses();
     usePageFilters.mockClear();
 
-    ProjectsStore.loadInitialData([Project({name: 'Awesome API', slug: 'awesome-api'})]);
+    ProjectsStore.loadInitialData([
+      Project({
+        name: 'Awesome API',
+        slug: 'awesome-api',
+        features: ['span-metrics-extraction'],
+      }),
+    ]);
 
     usePageFilters.mockImplementation(() => ({
       selection: PageFilters({projects: [2]}),
@@ -31,47 +37,17 @@ describe('NoDataMessage', () => {
     }));
   });
 
-  it('does not show anything while data is loading', async function () {
-    MockApiClient.addMockResponse({
-      url: '/organizations/org-slug/sdk-updates/',
-      body: [],
-    });
-
-    const eventsMock = MockApiClient.addMockResponse({
-      url: '/organizations/org-slug/events/',
-      body: {
-        data: [],
-      },
-    });
-
-    render(<NoDataMessage />);
-
-    await waitFor(() => expect(eventsMock).toHaveBeenCalled());
-
-    expect(
-      screen.queryByText(textWithMarkupMatcher('No queries found.'))
-    ).not.toBeInTheDocument();
-
-    await tick();
+  afterEach(() => {
+    ProjectsStore.reset();
   });
 
-  it('does not show anything if there is recent data', async function () {
+  it('does not show anything if there is recent data', function () {
     MockApiClient.addMockResponse({
       url: '/organizations/org-slug/sdk-updates/',
       body: [],
     });
 
-    const eventsMock = MockApiClient.addMockResponse({
-      url: '/organizations/org-slug/events/',
-      body: {
-        data: [{'project.id': 2, 'count()': 1}],
-      },
-    });
-
-    render(<NoDataMessage />);
-
-    await waitFor(() => expect(eventsMock).toHaveBeenCalled());
-    await tick(); // There is no visual indicator, this awaits the promise resolve
+    render(<NoDataMessage isDataAvailable />);
 
     expect(
       screen.queryByText(textWithMarkupMatcher('No queries found.'))
@@ -79,21 +55,13 @@ describe('NoDataMessage', () => {
   });
 
   it('shows a no data message if there is no recent data', async function () {
-    MockApiClient.addMockResponse({
+    const sdkMock = MockApiClient.addMockResponse({
       url: '/organizations/org-slug/sdk-updates/',
       body: [],
     });
 
-    const eventsMock = MockApiClient.addMockResponse({
-      url: '/organizations/org-slug/events/',
-      body: {
-        data: [],
-      },
-    });
-
-    render(<NoDataMessage />);
-
-    await waitFor(() => expect(eventsMock).toHaveBeenCalled());
+    render(<NoDataMessage isDataAvailable={false} />);
+    await waitFor(() => expect(sdkMock).toHaveBeenCalled());
     await tick(); // There is no visual indicator, this awaits the promise resolve
 
     expect(
@@ -101,7 +69,7 @@ describe('NoDataMessage', () => {
     ).toBeInTheDocument();
     expect(
       screen.queryByText(
-        textWithMarkupMatcher('You may also be missing data due to outdated SDKs')
+        textWithMarkupMatcher('You may be missing data due to outdated SDKs')
       )
     ).not.toBeInTheDocument();
   });
@@ -112,16 +80,8 @@ describe('NoDataMessage', () => {
       body: [ProjectSdkUpdates({projectId: '2'})],
     });
 
-    const eventsMock = MockApiClient.addMockResponse({
-      url: '/organizations/org-slug/events/',
-      body: {
-        data: [],
-      },
-    });
-
-    render(<NoDataMessage />);
+    render(<NoDataMessage isDataAvailable={false} />);
 
-    await waitFor(() => expect(eventsMock).toHaveBeenCalled());
     await waitFor(() => expect(sdkMock).toHaveBeenCalled());
     await tick(); // There is no visual indicator, this awaits the promise resolve
 
@@ -130,7 +90,7 @@ describe('NoDataMessage', () => {
     ).toBeInTheDocument();
     expect(
       screen.getByText(
-        textWithMarkupMatcher('You may also be missing data due to outdated SDKs')
+        textWithMarkupMatcher('You may be missing data due to outdated SDKs')
       )
     ).toBeInTheDocument();
 
@@ -139,4 +99,31 @@ describe('NoDataMessage', () => {
       '/organizations/org-slug/projects/awesome-api/'
     );
   });
+
+  it('shows a list of denylisted projects if any are are set even if data is available', async function () {
+    ProjectsStore.loadInitialData([
+      Project({
+        name: 'Awful API',
+        slug: 'awful-api',
+        features: [],
+      }),
+    ]);
+
+    render(<NoDataMessage isDataAvailable />);
+
+    await tick(); // There is no visual indicator, this awaits the promise resolve
+
+    expect(
+      screen.getByText(
+        textWithMarkupMatcher(
+          'Some of your projects have been omitted from query performance analysis'
+        )
+      )
+    ).toBeInTheDocument();
+
+    expect(screen.getAllByRole('link')[0]).toHaveAttribute(
+      'href',
+      '/organizations/org-slug/projects/awful-api/'
+    );
+  });
 });

+ 78 - 64
static/app/views/performance/database/noDataMessage.tsx

@@ -1,106 +1,120 @@
 import {Fragment} from 'react';
-import sumBy from 'lodash/sumBy';
 
+import {openHelpSearchModal} from 'sentry/actionCreators/modal';
+import {Button} from 'sentry/components/button';
 import ExternalLink from 'sentry/components/links/externalLink';
 import {t, tct} from 'sentry/locale';
+import {Project} from 'sentry/types';
 import useOrganization from 'sentry/utils/useOrganization';
 import usePageFilters from 'sentry/utils/usePageFilters';
 import {normalizeUrl} from 'sentry/utils/withDomainRequired';
-import {useIneligibleProjects} from 'sentry/views/performance/database/useIneligibleProjects';
-import {useProjectSpanMetricCounts} from 'sentry/views/starfish/queries/useProjectSpanMetricsCounts';
+import {useDenylistedProjects} from 'sentry/views/performance/database/useDenylistedProjects';
+import {useOutdatedSDKProjects} from 'sentry/views/performance/database/useOutdatedSDKProjects';
 
 interface Props {
   Wrapper?: React.ComponentType;
+  isDataAvailable?: boolean;
 }
 
 function DivWrapper(props) {
   return <div {...props} />;
 }
 
-export function NoDataMessage({Wrapper = DivWrapper}: Props) {
+export function NoDataMessage({Wrapper = DivWrapper, isDataAvailable}: Props) {
+  const organization = useOrganization();
   const {selection, isReady: pageFilterIsReady} = usePageFilters();
 
   const selectedProjectIds = selection.projects.map(projectId => projectId.toString());
 
-  const {data: projectSpanMetricsCounts, isLoading: areSpanMetricCountsLoading} =
-    useProjectSpanMetricCounts({
-      query: 'span.module:db',
-      statsPeriod: SAMPLE_STATS_PERIOD,
-      enabled: pageFilterIsReady,
+  const {projects: outdatedProjects, isFetching: areOutdatedProjectsFetching} =
+    useOutdatedSDKProjects({
       projectId: selectedProjectIds,
+      enabled: pageFilterIsReady && !isDataAvailable,
     });
 
-  const doesAnySelectedProjectHaveMetrics =
-    sumBy(projectSpanMetricsCounts, 'count()') > 0;
-
-  const {ineligibleProjects, isFetching: areIneligibleProjectsFetching} =
-    useIneligibleProjects({
+  const {projects: denylistedProjects, isFetching: areDenylistProjectsFetching} =
+    useDenylistedProjects({
       projectId: selectedProjectIds,
-      enabled: pageFilterIsReady && !doesAnySelectedProjectHaveMetrics,
     });
 
-  const organization = useOrganization();
+  const isDataFetching = areOutdatedProjectsFetching || areDenylistProjectsFetching;
 
-  const hasMoreIneligibleProjectsThanVisible =
-    ineligibleProjects.length > MAX_LISTED_PROJECTS;
-
-  if (areSpanMetricCountsLoading || areIneligibleProjectsFetching) {
+  if (isDataFetching) {
     return null;
   }
 
-  if (!projectSpanMetricsCounts) {
-    return null;
-  }
+  const hasAnyProblematicProjects =
+    (!areOutdatedProjectsFetching && outdatedProjects.length > 0) ||
+    (!areDenylistProjectsFetching && denylistedProjects.length > 0);
 
-  if (doesAnySelectedProjectHaveMetrics) {
+  if (isDataAvailable && !hasAnyProblematicProjects) {
     return null;
   }
 
-  const firstIneligibleProjects = ineligibleProjects.slice(0, MAX_LISTED_PROJECTS + 1);
-
   return (
     <Wrapper>
-      {t('No queries found.')}{' '}
-      {tct(
-        'Try updating your filters, or learn more about performance monitoring for queries in our [documentation:documentation].',
-        {
-          documentation: (
-            <ExternalLink href="https://docs.sentry.io/product/performance/queries/" />
-          ),
-        }
-      )}{' '}
-      {ineligibleProjects.length > 0 &&
-        tct('You may also be missing data due to outdated SDKs: [projectList]', {
-          documentation: (
-            <ExternalLink href="https://docs.sentry.io/product/performance/query-insights/" />
-          ),
-          projectList: (
-            <Fragment>
-              {firstIneligibleProjects.map((project, projectIndex) => {
-                return (
-                  <span key={project.id}>
-                    <a
-                      href={normalizeUrl(
-                        `/organizations/${organization.slug}/projects/${project.slug}/`
-                      )}
-                    >
-                      {project.name}
-                    </a>
-                    {projectIndex < firstIneligibleProjects.length - 1 && ', '}
-                  </span>
-                );
-              })}
-            </Fragment>
-          ),
-        })}
-      {hasMoreIneligibleProjectsThanVisible &&
-        tct(' and [count] more.', {
-          count: ineligibleProjects.length - MAX_LISTED_PROJECTS,
+      {!isDataAvailable &&
+        tct(
+          'No queries found. Try updating your filters, or learn more about performance monitoring for queries in our [documentation:documentation].',
+          {
+            documentation: (
+              <ExternalLink href="https://docs.sentry.io/product/performance/queries/" />
+            ),
+          }
+        )}{' '}
+      {outdatedProjects.length > 0 &&
+        tct('You may be missing data due to outdated SDKs: [projectList].', {
+          projectList: <ProjectList projects={outdatedProjects} />,
         })}{' '}
+      {denylistedProjects.length > 0 &&
+        tct(
+          'Some of your projects have been omitted from query performance analysis. Please [supportLink]. Omitted projects: [projectList].',
+          {
+            supportLink: (
+              <Button priority="link" onClick={() => openHelpSearchModal({organization})}>
+                {t('Contact Support')}
+              </Button>
+            ),
+            projectList: <ProjectList projects={denylistedProjects} />,
+          }
+        )}
     </Wrapper>
   );
 }
 
-const MAX_LISTED_PROJECTS = 3;
+interface ProjectListProps {
+  projects: Project[];
+  limit?: number;
+}
 
-const SAMPLE_STATS_PERIOD = '14d'; // The time period in which to check for any presence of span metrics
+function ProjectList({projects, limit = MAX_LISTED_PROJECTS}: ProjectListProps) {
+  const organization = useOrganization();
+
+  const visibleProjects = projects.slice(0, limit + 1);
+  const hasMoreProjectsThanVisible = projects.length > MAX_LISTED_PROJECTS;
+
+  return (
+    <Fragment>
+      {visibleProjects.slice(0, limit).map((project, projectIndex) => {
+        return (
+          <span key={project.id}>
+            <a
+              href={normalizeUrl(
+                `/organizations/${organization.slug}/projects/${project.slug}/`
+              )}
+            >
+              {project.name}
+            </a>
+            {projectIndex < visibleProjects.length - 1 && ', '}
+          </span>
+        );
+      })}
+      {hasMoreProjectsThanVisible &&
+        tct(' and [count] more.', {
+          count: projects.length - limit,
+        })}{' '}
+    </Fragment>
+  );
+}
+
+const MAX_LISTED_PROJECTS = 3;

+ 36 - 0
static/app/views/performance/database/useDenylistedProjects.tsx

@@ -0,0 +1,36 @@
+import useProjects from 'sentry/utils/useProjects';
+
+interface Options {
+  projectId?: string[];
+}
+
+/**
+ * Returns a list of projects that are not eligible for span metrics
+ * because they were denylisted.
+ *
+ * @param options Additional options
+ * @param options.projectId List of project IDs to check against. If omitted, checks all organization projects
+ * @returns List of projects
+ */
+export function useDenylistedProjects(options: Options = {}) {
+  const {projects, fetching} = useProjects();
+
+  const {projectId = []} = options;
+
+  const shouldCheckAllProjects = projectId.length === 0 || projectId.includes('-1');
+
+  const projectsToCheck = shouldCheckAllProjects
+    ? projects
+    : projects.filter(project => projectId.includes(project.id.toString()));
+
+  const denylistedProjects = projectsToCheck
+    .filter(project => {
+      return !project.features.includes('span-metrics-extraction');
+    })
+    .filter((item): item is NonNullable<typeof item> => Boolean(item));
+
+  return {
+    projects: denylistedProjects,
+    isFetching: fetching,
+  };
+}

+ 5 - 3
static/app/views/performance/database/useIneligibleProjects.tsx → static/app/views/performance/database/useOutdatedSDKProjects.tsx

@@ -1,3 +1,5 @@
+import uniqBy from 'lodash/uniqBy';
+
 import ProjectsStore from 'sentry/stores/projectsStore';
 import {useOrganizationSDKUpdates} from 'sentry/utils/useOrganizationSDKUpdates';
 import {semverCompare} from 'sentry/utils/versions';
@@ -16,11 +18,11 @@ interface Options {
  * @param options.projectId List of project IDs to check against. If omitted, checks all organization projects
  * @returns List of projects
  */
-export function useIneligibleProjects(options?: Options) {
+export function useOutdatedSDKProjects(options?: Options) {
   const response = useOrganizationSDKUpdates(options ?? {});
   const {data: availableUpdates} = response;
 
-  const ineligibleProjects = (availableUpdates ?? [])
+  const projects = (availableUpdates ?? [])
     .filter(update => {
       const platform = removeFlavorFromSDKName(update.sdkName);
       const minimumRequiredVersion = MIN_SDK_VERSION_BY_PLATFORM[platform];
@@ -41,7 +43,7 @@ export function useIneligibleProjects(options?: Options) {
 
   return {
     ...response,
-    ineligibleProjects,
+    projects: uniqBy(projects, 'id'),
   };
 }
 

+ 1 - 1
static/app/views/performance/landing/widgets/components/selectableList.tsx

@@ -96,7 +96,7 @@ export function TimeSpentInDatabaseWidgetEmptyStateWarning() {
     <StyledEmptyStateWarning>
       <PrimaryMessage>{t('No results found')}</PrimaryMessage>
       <SecondaryMessage>
-        <NoDataMessage Wrapper={Fragment} />
+        <NoDataMessage Wrapper={Fragment} isDataAvailable={false} />
       </SecondaryMessage>
     </StyledEmptyStateWarning>
   );