feat(codecov): Add tooltips to stacktrace codecov coverage (#44814)

static/app/components/events/interfaces/frame/codecovLegend.spec.tsx

@@ -1,53 +0,0 @@
-import {render, screen} from 'sentry-test/reactTestingLibrary';
-
-import ProjectsStore from 'sentry/stores/projectsStore';
-import {CodecovStatusCode, Frame} from 'sentry/types';
-
-import {CodecovLegend} from './codecovLegend';
-
-describe('Frame - Codecov Legend', function () {
-  const organization = TestStubs.Organization({
-    features: ['codecov-stacktrace-integration'],
-    codecovAccess: true,
-  });
-  const platform = 'python';
-  const project = TestStubs.Project({});
-  const event = TestStubs.Event({
-    projectID: project.id,
-    release: TestStubs.Release({lastCommit: TestStubs.Commit()}),
-    platform,
-  });
-  const integration = TestStubs.GitHubIntegration();
-  const repo = TestStubs.Repository({integrationId: integration.id});
-
-  const frame = {filename: '/sentry/app.py', lineNo: 233} as Frame;
-  const config = TestStubs.RepositoryProjectPathConfig({project, repo, integration});
-
-  beforeEach(function () {
-    jest.clearAllMocks();
-    MockApiClient.clearMockResponses();
-    ProjectsStore.loadInitialData([project]);
-  });
-
-  it('should render codecov legend', async function () {
-    MockApiClient.addMockResponse({
-      url: `/projects/${organization.slug}/${project.slug}/stacktrace-link/`,
-      body: {
-        config,
-        sourceUrl: null,
-        integrations: [integration],
-        codecov: {status: CodecovStatusCode.COVERAGE_EXISTS},
-      },
-    });
-
-    render(<CodecovLegend event={event} frame={frame} organization={organization} />, {
-      context: TestStubs.routerContext(),
-      organization,
-      project,
-    });
-
-    expect(await screen.findByText('Covered')).toBeInTheDocument();
-    expect(await screen.findByText('Uncovered')).toBeInTheDocument();
-    expect(await screen.findByText('Partial')).toBeInTheDocument();
-  });
-});

static/app/components/events/interfaces/frame/context.spec.tsx

@@ -3,7 +3,7 @@ import {render} from 'sentry-test/reactTestingLibrary';
 import ProjectsStore from 'sentry/stores/projectsStore';
 import {Coverage, Frame, LineCoverage} from 'sentry/types';
 
-import Context, {getCoverageColorClass} from './context';
+import Context, {getLineCoverage} from './context';
 
 describe('Frame - Context', function () {
   const org = TestStubs.Organization();
@@ -34,11 +34,9 @@ describe('Frame - Context', function () {
     [234, Coverage.NOT_COVERED],
   ];
 
-  const primaryLineNumber = 233;
-
-  it('converts coverage data to the right colors', function () {
-    expect(getCoverageColorClass(lines, lineCoverage, primaryLineNumber)).toEqual([
-      ['partial', 'covered', 'active', 'uncovered'],
+  it("gets coverage data for the frame's lines", function () {
+    expect(getLineCoverage(lines, lineCoverage)).toEqual([
+      [Coverage.PARTIAL, Coverage.COVERED, undefined, Coverage.NOT_COVERED],
       true,
     ]);
   });

static/app/components/events/interfaces/frame/context.tsx

@@ -49,46 +49,19 @@ type Props = {
   registersMeta?: Record<any, any>;
 };
 
-export function getCoverageColorClass(
+export function getLineCoverage(
   lines: [number, string][],
-  lineCov: LineCoverage[],
-  activeLineNo: number
-): [Array<string>, boolean] {
-  const lineCoverage = keyBy(lineCov, 0);
-  let hasCoverage = false;
-  const lineColors = lines.map(([lineNo]) => {
-    const coverage = lineCoverage[lineNo]
-      ? lineCoverage[lineNo][1]
-      : Coverage.NOT_APPLICABLE;
-
-    let color = '';
-    switch (coverage) {
-      case Coverage.COVERED:
-        color = 'covered';
-        break;
-      case Coverage.NOT_COVERED:
-        color = 'uncovered';
-        break;
-      case Coverage.PARTIAL:
-        color = 'partial';
-        break;
-      case Coverage.NOT_APPLICABLE:
-      // fallthrough
-      default:
-        break;
-    }
-
-    if (color !== '') {
-      hasCoverage = true;
-    }
-
-    if (activeLineNo !== lineNo) {
-      return color;
-    }
-    return color === '' ? 'active' : `active ${color}`;
-  });
+  lineCov: LineCoverage[]
+): [Array<Coverage | undefined>, boolean] {
+  const keyedCoverage = keyBy(lineCov, 0);
+  const lineCoverage = lines.map<Coverage | undefined>(
+    ([lineNo]) => keyedCoverage[lineNo]?.[1]
+  );
+  const hasCoverage = lineCoverage.some(
+    coverage => coverage !== Coverage.NOT_APPLICABLE && coverage !== undefined
+  );
 
-  return [lineColors, hasCoverage];
+  return [lineCoverage, hasCoverage];
 }
 
 const Context = ({
@@ -131,16 +104,20 @@ const Context = ({
     }
   );
 
+  /**
+   * frame.lineNo is the highlighted frame in the middle of the context
+   */
+  const activeLineNumber = frame.lineNo;
   const contextLines = isExpanded
     ? frame?.context
-    : frame?.context?.filter(l => l[0] === frame.lineNo);
+    : frame?.context?.filter(l => l[0] === activeLineNumber);
 
   const hasCoverageData =
     !isLoading && data?.codecov?.status === CodecovStatusCode.COVERAGE_EXISTS;
 
-  const [lineColors = [], hasCoverage] =
-    hasCoverageData && data!.codecov?.lineCoverage && !!frame.lineNo! && contextLines
-      ? getCoverageColorClass(contextLines, data!.codecov?.lineCoverage, frame.lineNo)
+  const [lineCoverage = [], hasCoverage] =
+    hasCoverageData && data!.codecov?.lineCoverage && !!activeLineNumber! && contextLines
+      ? getLineCoverage(contextLines, data!.codecov?.lineCoverage)
       : [];
 
   useRouteAnalyticsParams(
@@ -181,16 +158,16 @@ const Context = ({
 
       {frame.context &&
         contextLines.map((line, index) => {
-          const isActive = frame.lineNo === line[0];
+          const isActive = activeLineNumber === line[0];
           const hasComponents = isActive && components.length > 0;
           const showStacktraceLink = hasStacktraceLink && isActive;
 
           return (
-            <StyledContextLine
+            <ContextLine
               key={index}
               line={line}
               isActive={isActive}
-              colorClass={lineColors[index] ?? ''}
+              coverage={lineCoverage[index]}
             >
               {hasComponents && (
                 <ErrorBoundary mini>
@@ -212,7 +189,7 @@ const Context = ({
                   />
                 </ErrorBoundary>
               )}
-            </StyledContextLine>
+            </ContextLine>
           );
         })}
 
@@ -247,67 +224,6 @@ const StyledIconFlag = styled(IconFlag)`
   margin-right: ${space(1)};
 `;
 
-const StyledContextLine = styled(ContextLine)`
-  background: inherit;
-  z-index: 1000;
-  list-style: none;
-
-  &::marker {
-    content: none;
-  }
-
-  &:before {
-    content: counter(frame);
-    counter-increment: frame;
-    text-align: center;
-    padding-left: ${space(3)};
-    padding-right: ${space(1.5)};
-    margin-right: ${space(1.5)};
-    display: inline-block;
-    height: 24px;
-    background: transparent;
-    z-index: 1;
-    min-width: 58px;
-    border-right-style: solid;
-    border-right-color: transparent;
-  }
-
-  &.covered:before {
-    background: ${p => p.theme.green100};
-    border-right-color: ${p => p.theme.green300};
-  }
-
-  &.uncovered:before {
-    background: ${p => p.theme.red100};
-  }
-
-  &.partial:before {
-    background: ${p => p.theme.yellow100};
-    border-right-style: dashed;
-    border-right-color: ${p => p.theme.yellow300};
-  }
-
-  &.active {
-    background: ${p => p.theme.stacktraceActiveBackground};
-    color: ${p => p.theme.stacktraceActiveText};
-  }
-
-  &.active.partial:before {
-    mix-blend-mode: screen;
-    background: ${p => p.theme.yellow200};
-  }
-
-  &.active.covered:before {
-    mix-blend-mode: screen;
-    background: ${p => p.theme.green200};
-  }
-
-  &.active.uncovered:before {
-    mix-blend-mode: screen;
-    background: ${p => p.theme.red200};
-  }
-`;
-
 const Wrapper = styled('ol')<{startLineNo: number}>`
   counter-reset: frame ${p => p.startLineNo - 1};
 

static/app/components/events/interfaces/frame/contextLine.tsx

@@ -1,44 +1,128 @@
-import {Fragment} from 'react';
 import styled from '@emotion/styled';
 import classNames from 'classnames';
 
+import {Tooltip} from 'sentry/components/tooltip';
+import {t} from 'sentry/locale';
+import {space} from 'sentry/styles/space';
+import {Coverage} from 'sentry/types';
+
 interface Props {
-  colorClass: string;
   isActive: boolean;
-  line: [number, string];
+  line: [lineNo: number, content: string];
   children?: React.ReactNode;
-  className?: string;
+  coverage?: Coverage | '';
 }
 
-const ContextLine = function ({line, isActive, children, className, colorClass}: Props) {
+const coverageText: Record<Coverage, string | undefined> = {
+  [Coverage.NOT_COVERED]: t('Uncovered'),
+  [Coverage.COVERED]: t('Covered'),
+  [Coverage.PARTIAL]: t('Partially Covered'),
+  [Coverage.NOT_APPLICABLE]: undefined,
+};
+const coverageClass: Record<Coverage, string | undefined> = {
+  [Coverage.NOT_COVERED]: 'uncovered',
+  [Coverage.COVERED]: 'covered',
+  [Coverage.PARTIAL]: 'partial',
+  [Coverage.NOT_APPLICABLE]: undefined,
+};
+
+function ContextLine({line, isActive, children, coverage = ''}: Props) {
   let lineWs = '';
   let lineCode = '';
   if (typeof line[1] === 'string') {
     [, lineWs, lineCode] = line[1].match(/^(\s*)(.*?)$/m)!;
   }
-  const Component = !children ? Fragment : Context;
-  const hasCoverage = colorClass !== '';
 
   return (
-    <li
+    <StyledLi
       className={classNames(
-        className,
         'expandable',
-        hasCoverage ? colorClass : {active: isActive}
+        coverageClass[coverage],
+        isActive ? 'active' : ''
       )}
-      key={line[0]}
     >
-      <Component>
-        <span className="ws">{lineWs}</span>
-        <span className="contextline">{lineCode}</span>
-      </Component>
+      <LineContent>
+        <Tooltip skipWrapper title={coverageText[coverage]} delay={200}>
+          <div className="line-number">{line[0]}</div>
+        </Tooltip>
+        <div>
+          <span className="ws">{lineWs}</span>
+          <span className="contextline">{lineCode}</span>
+        </div>
+      </LineContent>
       {children}
-    </li>
+    </StyledLi>
   );
-};
+}
 
 export default ContextLine;
 
-const Context = styled('div')`
-  display: inline;
+const StyledLi = styled('li')`
+  background: inherit;
+  z-index: 1000;
+  list-style: none;
+
+  &::marker {
+    content: none;
+  }
+
+  .line-number {
+    display: flex;
+    align-items: center;
+    flex-direction: row;
+    flex-wrap: nowrap;
+    justify-content: end;
+    height: 100%;
+    text-align: right;
+    padding-left: ${space(2)};
+    padding-right: ${space(2)};
+    margin-right: ${space(1.5)};
+    background: transparent;
+    z-index: 1;
+    min-width: 58px;
+    border-right-style: solid;
+    border-right-color: transparent;
+    user-select: none;
+  }
+
+  &.covered .line-number {
+    background: ${p => p.theme.green100};
+  }
+
+  &.uncovered .line-number {
+    background: ${p => p.theme.red100};
+    border-right-color: ${p => p.theme.red300};
+  }
+
+  &.partial .line-number {
+    background: ${p => p.theme.yellow100};
+    border-right-style: dashed;
+    border-right-color: ${p => p.theme.yellow300};
+  }
+
+  &.active {
+    background: ${p => p.theme.stacktraceActiveBackground};
+    color: ${p => p.theme.stacktraceActiveText};
+  }
+
+  &.active.partial .line-number {
+    mix-blend-mode: screen;
+    background: ${p => p.theme.yellow200};
+  }
+
+  &.active.covered .line-number {
+    mix-blend-mode: screen;
+    background: ${p => p.theme.green200};
+  }
+
+  &.active.uncovered .line-number {
+    mix-blend-mode: screen;
+    background: ${p => p.theme.red300};
+  }
+`;
+
+// TODO(scttcper): The parent component should be a grid, currently has too many other children
+const LineContent = styled('div')`
+  display: grid;
+  grid-template-columns: 58px 1fr;
 `;

static/app/components/events/interfaces/frame/line.tsx

@@ -24,7 +24,6 @@ import DebugImage from '../debugMeta/debugImage';
 import {combineStatus} from '../debugMeta/utils';
 import {SymbolicatorStatus} from '../types';
 
-import {CodecovLegend} from './codecovLegend';
 import Context from './context';
 import DefaultTitle from './defaultTitle';
 import PackageLink from './packageLink';
@@ -400,21 +399,8 @@ export class Line extends Component<Props, State> {
     });
     const props = {className};
 
-    const shouldShowCodecovLegend =
-      this.props.organization?.features.includes('codecov-stacktrace-integration') &&
-      this.props.organization?.codecovAccess &&
-      !this.props.nextFrame &&
-      this.state.isExpanded;
-
     return (
       <StyledLi data-test-id="line" {...props}>
-        {shouldShowCodecovLegend && (
-          <CodecovLegend
-            event={this.props.event}
-            frame={this.props.data}
-            organization={this.props.organization}
-          />
-        )}
         {this.renderLine()}
         <Context
           frame={data}

static/app/components/events/interfaces/frame/lineV2/index.tsx

@@ -2,12 +2,10 @@ import {useState} from 'react';
 import styled from '@emotion/styled';
 import classNames from 'classnames';
 
-import {CodecovLegend} from 'sentry/components/events/interfaces/frame/codecovLegend';
 import ListItem from 'sentry/components/list/listItem';
 import StrictClick from 'sentry/components/strictClick';
 import {PlatformType, SentryAppComponent} from 'sentry/types';
 import {Event} from 'sentry/types/event';
-import useOrganization from 'sentry/utils/useOrganization';
 import withSentryAppComponents from 'sentry/utils/withSentryAppComponents';
 
 import Context from '../context';
@@ -78,7 +76,6 @@ function Line({
    of the stack trace / exception */
   const platform = getPlatform(frame.platform, props.platform ?? 'other') as PlatformType;
   const leadsToApp = !frame.inApp && ((nextFrame && nextFrame.inApp) || !nextFrame);
-  const organization = useOrganization();
 
   const expandable =
     !leadsToApp || includeSystemFrames
@@ -174,17 +171,8 @@ function Line({
     'leads-to-app': leadsToApp,
   });
 
-  const shouldShowCodecovLegend =
-    organization.features.includes('codecov-stacktrace-integration') &&
-    organization.codecovAccess &&
-    !nextFrame &&
-    isExpanded;
-
   return (
     <StyleListItem className={className} data-test-id="stack-trace-frame">
-      {shouldShowCodecovLegend && (
-        <CodecovLegend event={event} frame={frame} organization={organization} />
-      )}
       <StrictClick onClick={expandable ? toggleContext : undefined}>
         {renderLine()}
       </StrictClick>

static/app/components/events/interfaces/frame/openInContextLine.tsx

@@ -53,7 +53,7 @@ const OpenInContextLine = ({lineNo, filename, components}: Props) => {
 
 export {OpenInContextLine};
 
-export const OpenInContainer = styled('div')<{columnQuantity: number}>`
+const OpenInContainer = styled('div')<{columnQuantity: number}>`
   position: relative;
   z-index: 1;
   display: grid;

static/app/components/events/interfaces/frame/stacktraceLink.tsx

@@ -14,15 +14,13 @@ import ExternalLink from 'sentry/components/links/externalLink';
 import Link from 'sentry/components/links/link';
 import Placeholder from 'sentry/components/placeholder';
 import type {PlatformKey} from 'sentry/data/platformCategories';
-import {IconCircle, IconCircleFill, IconClose, IconWarning} from 'sentry/icons';
+import {IconClose, IconWarning} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import {
   CodecovStatusCode,
-  Coverage,
   Event,
   Frame,
-  LineCoverage,
   Organization,
   Project,
   StacktraceLinkResult,
@@ -40,7 +38,6 @@ import useApi from 'sentry/utils/useApi';
 import useOrganization from 'sentry/utils/useOrganization';
 import useProjects from 'sentry/utils/useProjects';
 
-import {OpenInContainer} from './openInContextLine';
 import StacktraceLinkModal from './stacktraceLinkModal';
 import useStacktraceLink from './useStacktraceLink';
 
@@ -97,7 +94,7 @@ function StacktraceLinkSetup({organization, project, event}: StacktraceLinkSetup
   };
 
   return (
-    <CodeMappingButtonContainer columnQuantity={2}>
+    <StacktraceLinkWrapper>
       <StyledLink to={`/settings/${organization.slug}/integrations/`}>
         <StyledIconWrapper>{getIntegrationIcon('github', 'sm')}</StyledIconWrapper>
         {t('Add the GitHub or GitLab integration to jump straight to your source code')}
@@ -105,7 +102,7 @@ function StacktraceLinkSetup({organization, project, event}: StacktraceLinkSetup
       <CloseButton priority="link" onClick={dismissPrompt}>
         <IconClose size="xs" aria-label={t('Close')} />
       </CloseButton>
-    </CodeMappingButtonContainer>
+    </StacktraceLinkWrapper>
   );
 }
 
@@ -125,55 +122,14 @@ function shouldshowCodecovFeatures(
 
 interface CodecovLinkProps {
   event: Event;
-  lineNo: number | null;
   organization: Organization;
   coverageUrl?: string;
-  lineCoverage?: LineCoverage[];
   status?: CodecovStatusCode;
 }
 
-function getCoverageIcon(lineCoverage, lineNo) {
-  const covIndex = lineCoverage.findIndex(line => line[0] === lineNo);
-  if (covIndex === -1) {
-    return null;
-  }
-  switch (lineCoverage[covIndex][1]) {
-    case Coverage.COVERED:
-      return (
-        <CoverageIcon>
-          <IconCircleFill size="xs" color="green100" style={{position: 'absolute'}} />
-          <IconCircle size="xs" color="green300" />
-          {t('Covered')}
-        </CoverageIcon>
-      );
-    case Coverage.PARTIAL:
-      return (
-        <CoverageIcon>
-          <IconCircleFill size="xs" color="yellow100" style={{position: 'absolute'}} />
-          <IconCircle size="xs" color="yellow300" />
-          {t('Partially Covered')}
-        </CoverageIcon>
-      );
-    case Coverage.NOT_COVERED:
-      return (
-        <CodecovContainer>
-          <CoverageIcon>
-            <IconCircleFill size="xs" color="red100" style={{position: 'absolute'}} />
-            <IconCircle size="xs" color="red300" />
-          </CoverageIcon>
-          {t('Not Covered')}
-        </CodecovContainer>
-      );
-    default:
-      return null;
-  }
-}
-
 function CodecovLink({
   coverageUrl,
   status = CodecovStatusCode.COVERAGE_EXISTS,
-  lineCoverage,
-  lineNo,
   organization,
   event,
 }: CodecovLinkProps) {
@@ -186,31 +142,25 @@ function CodecovLink({
     );
   }
 
-  if (status === CodecovStatusCode.COVERAGE_EXISTS) {
-    if (!coverageUrl || !lineCoverage || !lineNo) {
-      return null;
-    }
+  if (status !== CodecovStatusCode.COVERAGE_EXISTS || !coverageUrl) {
+    return null;
+  }
 
-    const onOpenCodecovLink = () => {
-      trackIntegrationAnalytics(StacktraceLinkEvents.CODECOV_LINK_CLICKED, {
-        view: 'stacktrace_issue_details',
-        organization,
-        group_id: event.groupID ? parseInt(event.groupID, 10) : -1,
-        ...getAnalyticsDataForEvent(event),
-      });
-    };
+  const onOpenCodecovLink = () => {
+    trackIntegrationAnalytics(StacktraceLinkEvents.CODECOV_LINK_CLICKED, {
+      view: 'stacktrace_issue_details',
+      organization,
+      group_id: event.groupID ? parseInt(event.groupID, 10) : -1,
+      ...getAnalyticsDataForEvent(event),
+    });
+  };
 
-    return (
-      <CodecovContainer>
-        {getCoverageIcon(lineCoverage, lineNo)}
-        <OpenInLink href={coverageUrl} openInNewTab onClick={onOpenCodecovLink}>
-          <StyledIconWrapper>{getIntegrationIcon('codecov', 'sm')}</StyledIconWrapper>
-          {t('Open in Codecov')}
-        </OpenInLink>
-      </CodecovContainer>
-    );
-  }
-  return null;
+  return (
+    <OpenInLink href={coverageUrl} openInNewTab onClick={onOpenCodecovLink}>
+      <StyledIconWrapper>{getIntegrationIcon('codecov', 'sm')}</StyledIconWrapper>
+      {t('Open in Codecov')}
+    </OpenInLink>
+  );
 }
 
 interface StacktraceLinkProps {
@@ -306,16 +256,16 @@ export function StacktraceLink({frame, event, line}: StacktraceLinkProps) {
 
   if (isLoading || !match) {
     return (
-      <CodeMappingButtonContainer columnQuantity={2}>
-        <Placeholder height="24px" width="60px" />
-      </CodeMappingButtonContainer>
+      <StacktraceLinkWrapper>
+        <Placeholder height="24px" width="120px" />
+      </StacktraceLinkWrapper>
     );
   }
 
   // Match found - display link to source
   if (match.config && match.sourceUrl) {
     return (
-      <CodeMappingButtonContainer columnQuantity={2}>
+      <StacktraceLinkWrapper>
         <OpenInLink
           onClick={onOpenLink}
           href={`${match!.sourceUrl}#L${frame.lineNo}`}
@@ -330,13 +280,11 @@ export function StacktraceLink({frame, event, line}: StacktraceLinkProps) {
           <CodecovLink
             coverageUrl={`${match.codecov?.coverageUrl}#L${frame.lineNo}`}
             status={match.codecov?.status}
-            lineCoverage={match.codecov?.lineCoverage}
-            lineNo={frame.lineNo}
             organization={organization}
             event={event}
           />
         )}
-      </CodeMappingButtonContainer>
+      </StacktraceLinkWrapper>
     );
   }
 
@@ -359,7 +307,7 @@ export function StacktraceLink({frame, event, line}: StacktraceLinkProps) {
       ['github', 'gitlab'].includes(integration.provider?.key)
     );
     return (
-      <CodeMappingButtonContainer columnQuantity={2}>
+      <StacktraceLinkWrapper>
         <FixMappingButton
           priority="link"
           icon={
@@ -392,7 +340,7 @@ export function StacktraceLink({frame, event, line}: StacktraceLinkProps) {
         >
           {t('Tell us where your source code is')}
         </FixMappingButton>
-      </CodeMappingButtonContainer>
+      </StacktraceLinkWrapper>
     );
   }
 
@@ -407,8 +355,15 @@ export function StacktraceLink({frame, event, line}: StacktraceLinkProps) {
   );
 }
 
-export const CodeMappingButtonContainer = styled(OpenInContainer)`
-  justify-content: space-between;
+const StacktraceLinkWrapper = styled('div')`
+  display: flex;
+  gap: ${space(2)};
+  color: ${p => p.theme.subText};
+  background-color: ${p => p.theme.background};
+  font-family: ${p => p.theme.text.family};
+  border-bottom: 1px solid ${p => p.theme.border};
+  padding: ${space(0.25)} ${space(3)};
+  box-shadow: ${p => p.theme.dropShadowLight};
   min-height: 28px;
 `;
 
@@ -447,14 +402,3 @@ const CodecovWarning = styled('div')`
   gap: ${space(0.75)};
   align-items: center;
 `;
-
-const CodecovContainer = styled('span')`
-  display: flex;
-  gap: ${space(0.75)};
-`;
-
-const CoverageIcon = styled('span')`
-  display: flex;
-  gap: ${space(0.75)};
-  align-items: center;
-`;

static/less/group-detail.less

@@ -543,6 +543,7 @@ div.traceback > ul {
     &.expanded {
       .expandable {
         height: auto;
+        max-width: 100%;
       }
     }