Browse Source

feat(quick-trace): Only show one configure prompt (#25629)

It is possible that both the configure suspect commits prompt and configure
distributed tracing prompt can appear at the same time. This change ensures that
only one will appear at a time by segmenting events into two groups by event id.
This way, configure suspect commits will only appear in one group and configure
distributed tracing will only appear in the other.
Tony Xiao 3 years ago
parent
commit
5493a13a6b

+ 15 - 16
static/app/components/events/eventCauseEmpty.tsx

@@ -12,10 +12,11 @@ import {DataSection} from 'app/components/events/styles';
 import {Panel} from 'app/components/panels';
 import {t} from 'app/locale';
 import space from 'app/styles/space';
-import {Commit, Organization, Project, PromptActivity, RepositoryStatus} from 'app/types';
+import {Commit, Organization, Project, RepositoryStatus} from 'app/types';
+import {Event} from 'app/types/event';
 import {trackAdhocEvent, trackAnalyticsEvent} from 'app/utils/analytics';
 import getDynamicText from 'app/utils/getDynamicText';
-import {snoozedDays} from 'app/utils/promptsActivity';
+import {promptCanShow, promptIsDismissed} from 'app/utils/promptIsDismissed';
 import withApi from 'app/utils/withApi';
 
 const EXAMPLE_COMMITS = ['dec0de', 'de1e7e', '5ca1ed'];
@@ -76,6 +77,8 @@ const DUMMY_COMMIT: Commit = {
   message: t('This example commit broke something'),
 };
 
+const SUSPECT_COMMITS_FEATURE = 'suspect_commits';
+
 type ClickPayload = {
   action: 'snoozed' | 'dismissed';
   eventKey: string;
@@ -83,6 +86,7 @@ type ClickPayload = {
 };
 
 type Props = {
+  event: Event;
   organization: Organization;
   project: Project;
   api: Client;
@@ -117,25 +121,20 @@ class EventCauseEmpty extends React.Component<Props, State> {
   }
 
   async fetchData() {
-    const {api, project, organization} = this.props;
+    const {api, event, project, organization} = this.props;
+
+    if (!promptCanShow(SUSPECT_COMMITS_FEATURE, event.eventID)) {
+      this.setState({shouldShow: false});
+      return;
+    }
 
     const data = await promptsCheck(api, {
       projectId: project.id,
       organizationId: organization.id,
-      feature: 'suspect_commits',
+      feature: SUSPECT_COMMITS_FEATURE,
     });
 
-    this.setState({shouldShow: this.shouldShow(data ?? {})});
-  }
-
-  shouldShow({snoozedTime, dismissedTime}: PromptActivity) {
-    if (dismissedTime) {
-      return false;
-    }
-    if (snoozedTime) {
-      return snoozedDays(snoozedTime) > 7;
-    }
-    return true;
+    this.setState({shouldShow: !promptIsDismissed(data ?? {}, 7)});
   }
 
   handleClick({action, eventKey, eventName}: ClickPayload) {
@@ -144,7 +143,7 @@ class EventCauseEmpty extends React.Component<Props, State> {
     const data = {
       projectId: project.id,
       organizationId: organization.id,
-      feature: 'suspect_commits',
+      feature: SUSPECT_COMMITS_FEATURE,
       status: action,
     };
     promptsUpdate(api, data).then(() => this.setState({shouldShow: false}));

+ 5 - 1
static/app/components/events/eventEntries.tsx

@@ -351,7 +351,11 @@ class EventEntries extends React.Component<Props, State> {
         {!isShare &&
           isNotSharedOrganization(organization) &&
           (showExampleCommit ? (
-            <EventCauseEmpty organization={organization} project={project} />
+            <EventCauseEmpty
+              event={event}
+              organization={organization}
+              project={project}
+            />
           ) : (
             <EventCause
               organization={organization}

+ 15 - 0
static/app/utils/promptIsDismissed.tsx

@@ -11,3 +11,18 @@ export const promptIsDismissed = (prompt: PromptData, daysToSnooze: number = 14)
   //check if it has been snoozed
   return !snoozedTime ? false : snoozedDays(snoozedTime) < daysToSnooze;
 };
+
+export function promptCanShow(prompt: string, uuid: string): boolean {
+  /**
+   * This is to ensure that only one of suspect_commits
+   * or distributed_tracing is shown at a given time.
+   */
+  const x = (parseInt(uuid.charAt(0), 16) || 0) % 2;
+  if (prompt === 'suspect_commits') {
+    return x === 1;
+  } else if (prompt === 'distributed_tracing') {
+    return x === 0;
+  } else {
+    return true;
+  }
+}

+ 16 - 8
static/app/views/organizationGroupDetails/eventQuickTrace.tsx

@@ -11,15 +11,19 @@ import {Panel} from 'app/components/panels';
 import {t} from 'app/locale';
 import space from 'app/styles/space';
 import {Group, Organization} from 'app/types';
+import {Event} from 'app/types/event';
 import {trackAnalyticsEvent} from 'app/utils/analytics';
 import {getDocsPlatform} from 'app/utils/docs';
-import {promptIsDismissed} from 'app/utils/promptIsDismissed';
+import {promptCanShow, promptIsDismissed} from 'app/utils/promptIsDismissed';
 import withApi from 'app/utils/withApi';
 
+const DISTRIBUTED_TRACING_FEATURE = 'distributed_tracing';
+
 type Props = {
   api: Client;
   group: Group;
   organization: Organization;
+  event: Event;
 };
 
 type State = {
@@ -36,13 +40,18 @@ class EventQuickTrace extends React.Component<Props, State> {
   }
 
   async fetchData() {
-    const {api, group, organization} = this.props;
+    const {api, event, group, organization} = this.props;
     const {project} = group;
 
+    if (!promptCanShow(DISTRIBUTED_TRACING_FEATURE, event.eventID)) {
+      this.setState({shouldShow: false});
+      return;
+    }
+
     const data = await promptsCheck(api, {
       projectId: project.id,
       organizationId: organization.id,
-      feature: 'distributed_tracing',
+      feature: DISTRIBUTED_TRACING_FEATURE,
     });
 
     this.setState({shouldShow: !promptIsDismissed(data ?? {}, 30)});
@@ -67,7 +76,7 @@ class EventQuickTrace extends React.Component<Props, State> {
     const data = {
       projectId: project.id,
       organizationId: organization.id,
-      feature: 'distributed_tracing',
+      feature: DISTRIBUTED_TRACING_FEATURE,
       status: action,
     };
     promptsUpdate(api, data).then(() => this.setState({shouldShow: false}));
@@ -89,14 +98,13 @@ class EventQuickTrace extends React.Component<Props, State> {
     }
 
     const docsLink = this.createDocsLink();
-
     // if the platform does not support performance, do not show this prompt
     if (docsLink === null) {
       return null;
     }
 
     return (
-      <StyledPanel dashedBorder>
+      <ExampleQuickTracePanel dashedBorder>
         <div>
           <Header>{t('Configure Distributed Tracing')}</Header>
           <Description>
@@ -147,12 +155,12 @@ class EventQuickTrace extends React.Component<Props, State> {
             </Button>
           </ButtonBar>
         </ActionButtons>
-      </StyledPanel>
+      </ExampleQuickTracePanel>
     );
   }
 }
 
-const StyledPanel = styled(Panel)`
+const ExampleQuickTracePanel = styled(Panel)`
   display: grid;
   grid-template-columns: 1.5fr 1fr;
   grid-template-rows: auto max-content;

+ 1 - 0
static/app/views/organizationGroupDetails/eventToolbar.tsx

@@ -147,6 +147,7 @@ class GroupEventToolbar extends React.Component<Props> {
         </Tooltip>
         {hasQuickTraceView && !hasTraceContext && (
           <DistributedTracingPrompt
+            event={evt}
             group={this.props.group}
             organization={organization}
           />

+ 32 - 7
tests/js/spec/components/events/eventCauseEmpty.spec.jsx

@@ -16,8 +16,11 @@ describe('EventCauseEmpty', function () {
     platform: 'javascript',
     firstEvent: '2020-01-01T23:54:33.831199Z',
   });
+  const event = TestStubs.Event();
 
   beforeEach(function () {
+    jest.clearAllMocks();
+
     MockApiClient.clearMockResponses();
 
     MockApiClient.addMockResponse({
@@ -37,7 +40,7 @@ describe('EventCauseEmpty', function () {
 
   it('renders', async function () {
     const wrapper = mountWithTheme(
-      <EventCauseEmpty organization={organization} project={project} />,
+      <EventCauseEmpty event={event} organization={organization} project={project} />,
       routerContext
     );
 
@@ -54,9 +57,31 @@ describe('EventCauseEmpty', function () {
     });
   });
 
+  /**
+   * Want to alternate between showing the configure suspect commits prompt and
+   * the show configure distributed tracing prompt.
+   */
+  it('doesnt render when event id starts with even char', async function () {
+    const newEvent = {
+      ...event,
+      id: 'A',
+      eventID: 'ABCDEFABCDEFABCDEFABCDEFABCDEFAB',
+    };
+    const wrapper = mountWithTheme(
+      <EventCauseEmpty event={newEvent} organization={organization} project={project} />,
+      routerContext
+    );
+
+    await tick();
+    wrapper.update();
+
+    expect(wrapper.find('ExampleCommitPanel').exists()).toBe(false);
+    expect(trackAdhocEvent).not.toHaveBeenCalled();
+  });
+
   it('can be snoozed', async function () {
     const wrapper = mountWithTheme(
-      <EventCauseEmpty organization={organization} project={project} />,
+      <EventCauseEmpty event={event} organization={organization} project={project} />,
       routerContext
     );
 
@@ -102,7 +127,7 @@ describe('EventCauseEmpty', function () {
     });
 
     const wrapper = mountWithTheme(
-      <EventCauseEmpty organization={organization} project={project} />,
+      <EventCauseEmpty event={event} organization={organization} project={project} />,
       routerContext
     );
 
@@ -122,7 +147,7 @@ describe('EventCauseEmpty', function () {
     });
 
     const wrapper = mountWithTheme(
-      <EventCauseEmpty organization={organization} project={project} />,
+      <EventCauseEmpty event={event} organization={organization} project={project} />,
       routerContext
     );
 
@@ -134,7 +159,7 @@ describe('EventCauseEmpty', function () {
 
   it('can be dismissed', async function () {
     const wrapper = mountWithTheme(
-      <EventCauseEmpty organization={organization} project={project} />,
+      <EventCauseEmpty event={event} organization={organization} project={project} />,
       routerContext
     );
 
@@ -178,7 +203,7 @@ describe('EventCauseEmpty', function () {
     });
 
     const wrapper = mountWithTheme(
-      <EventCauseEmpty organization={organization} project={project} />,
+      <EventCauseEmpty event={event} organization={organization} project={project} />,
       routerContext
     );
 
@@ -190,7 +215,7 @@ describe('EventCauseEmpty', function () {
 
   it('can capture analytics on docs click', async function () {
     const wrapper = mountWithTheme(
-      <EventCauseEmpty organization={organization} project={project} />,
+      <EventCauseEmpty event={event} organization={organization} project={project} />,
       routerContext
     );
 

+ 223 - 0
tests/js/spec/views/organizationGroupDetails/eventQuickTrace.spec.jsx

@@ -0,0 +1,223 @@
+import React from 'react';
+import moment from 'moment';
+
+import {mountWithTheme} from 'sentry-test/enzyme';
+
+import {trackAnalyticsEvent} from 'app/utils/analytics';
+import EventQuickTrace from 'app/views/organizationGroupDetails/eventQuickTrace';
+
+jest.mock('app/utils/analytics');
+
+describe('EventQuickTrace', function () {
+  let putMock;
+  const routerContext = TestStubs.routerContext();
+  const organization = TestStubs.Organization();
+  const group = TestStubs.Group();
+  const event = {
+    ...TestStubs.Event(),
+    id: '2',
+    eventID: '21098765432109876543210987654321',
+  };
+
+  beforeEach(function () {
+    jest.clearAllMocks();
+
+    MockApiClient.clearMockResponses();
+
+    MockApiClient.addMockResponse({
+      method: 'GET',
+      url: '/prompts-activity/',
+      body: {},
+    });
+    putMock = MockApiClient.addMockResponse({
+      method: 'PUT',
+      url: '/prompts-activity/',
+    });
+  });
+
+  it('renders', async function () {
+    const wrapper = mountWithTheme(
+      <EventQuickTrace event={event} organization={organization} group={group} />,
+      routerContext
+    );
+
+    await tick();
+    wrapper.update();
+
+    expect(wrapper.find('ExampleQuickTracePanel').exists()).toBe(true);
+  });
+
+  /**
+   * Want to alternate between showing the configure suspect commits prompt and
+   * the show configure distributed tracing prompt.
+   */
+  it('doesnt render when event id starts with odd char', async function () {
+    const newEvent = {
+      ...event,
+      id: 'B',
+      eventID: 'BAFEDCBAFEDCBAFEDCBAFEDCBAFEDCBA',
+    };
+    const wrapper = mountWithTheme(
+      <EventQuickTrace event={newEvent} organization={organization} group={group} />,
+      routerContext
+    );
+
+    await tick();
+    wrapper.update();
+
+    expect(wrapper.find('ExampleQuickTracePanel').exists()).toBe(false);
+  });
+
+  it('doesnt render when the project platform doesnt support tracing', async function () {
+    const newGroup = {
+      ...group,
+      project: {
+        ...group.project,
+        platform: '',
+      },
+    };
+    const wrapper = mountWithTheme(
+      <EventQuickTrace event={event} organization={organization} group={newGroup} />,
+      routerContext
+    );
+
+    await tick();
+    wrapper.update();
+
+    expect(wrapper.find('ExampleQuickTracePanel').exists()).toBe(false);
+  });
+
+  it('can be snoozed', async function () {
+    const wrapper = mountWithTheme(
+      <EventQuickTrace event={event} organization={organization} group={group} />,
+      routerContext
+    );
+
+    await tick();
+    wrapper.update();
+
+    wrapper.find('button[aria-label="Snooze"]').first().simulate('click');
+
+    await tick();
+    await wrapper.update();
+
+    expect(putMock).toHaveBeenCalledWith(
+      '/prompts-activity/',
+      expect.objectContaining({
+        method: 'PUT',
+        data: {
+          organization_id: organization.id,
+          project_id: group.project.id,
+          feature: 'distributed_tracing',
+          status: 'snoozed',
+        },
+      })
+    );
+
+    expect(wrapper.find('ExampleQuickTracePanel').exists()).toBe(false);
+
+    expect(trackAnalyticsEvent).toHaveBeenCalledWith({
+      eventKey: 'quick_trace.missing_instrumentation.snoozed',
+      eventName: 'Quick Trace: Missing Instrumentation Snoozed',
+      organization_id: parseInt(organization.id, 10),
+      project_id: parseInt(group.project.id, 10),
+      platform: group.project.platform,
+    });
+  });
+
+  it('does not render when snoozed', async function () {
+    const snoozed_ts = moment().subtract(1, 'day').unix();
+
+    MockApiClient.addMockResponse({
+      method: 'GET',
+      url: '/prompts-activity/',
+      body: {data: {snoozed_ts}},
+    });
+
+    const wrapper = mountWithTheme(
+      <EventQuickTrace event={event} organization={organization} group={group} />,
+      routerContext
+    );
+
+    await tick();
+    wrapper.update();
+
+    expect(wrapper.find('ExampleQuickTracePanel').exists()).toBe(false);
+  });
+
+  it('can be dismissed', async function () {
+    const wrapper = mountWithTheme(
+      <EventQuickTrace event={event} organization={organization} group={group} />,
+      routerContext
+    );
+
+    await tick();
+    wrapper.update();
+
+    wrapper.find('button[aria-label="Dismiss"]').first().simulate('click');
+
+    await tick();
+    await wrapper.update();
+
+    expect(putMock).toHaveBeenCalledWith(
+      '/prompts-activity/',
+      expect.objectContaining({
+        method: 'PUT',
+        data: {
+          organization_id: organization.id,
+          project_id: group.project.id,
+          feature: 'distributed_tracing',
+          status: 'dismissed',
+        },
+      })
+    );
+
+    expect(wrapper.find('ExampleQuickTracePanel').exists()).toBe(false);
+
+    expect(trackAnalyticsEvent).toHaveBeenCalledWith({
+      eventKey: 'quick_trace.missing_instrumentation.dismissed',
+      eventName: 'Quick Trace: Missing Instrumentation Dismissed',
+      organization_id: parseInt(organization.id, 10),
+      project_id: parseInt(group.project.id, 10),
+      platform: group.project.platform,
+    });
+  });
+
+  it('does not render when dismissed', async function () {
+    MockApiClient.addMockResponse({
+      method: 'GET',
+      url: '/prompts-activity/',
+      body: {data: {dismissed_ts: moment().unix()}},
+    });
+
+    const wrapper = mountWithTheme(
+      <EventQuickTrace event={event} organization={organization} group={group} />,
+      routerContext
+    );
+
+    await tick();
+    wrapper.update();
+
+    expect(wrapper.find('ExampleQuickTracePanel').exists()).toBe(false);
+  });
+
+  it('can capture analytics on docs click', async function () {
+    const wrapper = mountWithTheme(
+      <EventQuickTrace event={event} organization={organization} group={group} />,
+      routerContext
+    );
+
+    await tick();
+    wrapper.update();
+
+    wrapper.find('[aria-label="Read the docs"]').first().simulate('click');
+
+    expect(trackAnalyticsEvent).toHaveBeenCalledWith({
+      eventKey: 'quick_trace.missing_instrumentation.docs',
+      eventName: 'Quick Trace: Missing Instrumentation Docs',
+      organization_id: parseInt(organization.id, 10),
+      project_id: parseInt(group.project.id, 10),
+      platform: group.project.platform,
+    });
+  });
+});