Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(trace-explorer): Render list of trace related project icons #73691

Merged
merged 6 commits into from
Jul 4, 2024
Merged

feat(trace-explorer): Render list of trace related project icons #73691

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b7468bd
feat(trace-explorer): Render list of trace related project icons
Zylphrex Jul 2, 2024
7dce111
add tests
Zylphrex Jul 3, 2024
8982636
Merge branch 'master' of github.com:getsentry/sentry into txiao/feat/…
Zylphrex Jul 3, 2024
6e12412
Merge branch 'master' of github.com:getsentry/sentry into txiao/feat/…
Zylphrex Jul 4, 2024
2a91a38
address PR comments
Zylphrex Jul 4, 2024
0bc9cb2
fix tests
Zylphrex Jul 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 51 additions & 14 deletions static/app/views/traces/content.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {space} from 'sentry/styles/space';
import type {PageFilters} from 'sentry/types/core';
import type {MetricAggregation, MRI} from 'sentry/types/metrics';
import type {Organization} from 'sentry/types/organization';
import {defined} from 'sentry/utils';
import {trackAnalytics} from 'sentry/utils/analytics';
import {browserHistory} from 'sentry/utils/browserHistory';
import {getUtcDateString} from 'sentry/utils/dates';
Expand All @@ -46,7 +47,9 @@ import * as ModuleLayout from 'sentry/views/insights/common/components/moduleLay
import {type Field, FIELDS, SORTS} from './data';
import {
BREAKDOWN_SLICES,
ProjectRenderer,
Description,
ProjectBadgeWrapper,
ProjectsRenderer,
SpanBreakdownSliceRenderer,
SpanDescriptionRenderer,
SpanIdRenderer,
Expand Down Expand Up @@ -301,6 +304,9 @@ export function Content() {
}

function TraceRow({defaultExpanded, trace}: {defaultExpanded; trace: TraceResult}) {
const {selection} = usePageFilters();
const {projects} = useProjects();

const [expanded, setExpanded] = useState<boolean>(defaultExpanded);
const [highlightedSliceName, _setHighlightedSliceName] = useState('');
const location = useLocation();
Expand All @@ -319,6 +325,41 @@ function TraceRow({defaultExpanded, trace}: {defaultExpanded; trace: TraceResult

const onClickExpand = useCallback(() => setExpanded(e => !e), [setExpanded]);

const selectedProjects = useMemo(() => {
const selectedProjectIds = new Set(
selection.projects.map(project => project.toString())
);
return new Set(
projects
.filter(project => selectedProjectIds.has(project.id))
.map(project => project.slug)
);
}, [projects, selection.projects]);

const traceProjects = useMemo(() => {
const seenProjects: Set<string> = new Set();

const leadingProjects: string[] = [];
const trailingProjects: string[] = [];

for (let i = 0; i < trace.breakdowns.length; i++) {
const project = trace.breakdowns[i].project;
if (!defined(project) || seenProjects.has(project)) {
continue;
}
seenProjects.add(project);

// Priotize projects that are selected in the page filters
if (selectedProjects.has(project)) {
leadingProjects.push(project);
} else {
trailingProjects.push(project);
}
}

return [...leadingProjects, ...trailingProjects];
}, [selectedProjects, trace]);

return (
<Fragment>
<StyledPanelItem align="center" center onClick={onClickExpand}>
Expand Down Expand Up @@ -348,9 +389,13 @@ function TraceRow({defaultExpanded, trace}: {defaultExpanded; trace: TraceResult
</StyledPanelItem>
<StyledPanelItem align="left" overflow>
<Description>
{trace.project ? (
<ProjectRenderer projectSlug={trace.project} hideName />
) : null}
<ProjectBadgeWrapper>
{traceProjects.length > 0 ? (
<ProjectsRenderer projectSlugs={traceProjects} />
) : trace.project ? (
<ProjectsRenderer projectSlugs={[trace.project]} />
) : null}
Zylphrex marked this conversation as resolved.
Show resolved Hide resolved
</ProjectBadgeWrapper>
{trace.name ? (
<WrappingText>{trace.name}</WrappingText>
) : (
Expand Down Expand Up @@ -796,13 +841,13 @@ const StyledPanel = styled(Panel)`
const TracePanelContent = styled('div')`
width: 100%;
display: grid;
grid-template-columns: repeat(1, min-content) auto repeat(2, min-content) 85px 112px 66px;
grid-template-columns: 116px auto repeat(2, min-content) 85px 112px 66px;
`;

const SpanPanelContent = styled('div')`
width: 100%;
display: grid;
grid-template-columns: repeat(1, min-content) auto repeat(1, min-content) 160px 85px;
grid-template-columns: 100px auto repeat(1, min-content) 160px 85px;
Comment on lines +846 to +852
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? doesn't this cover ids?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we dont use a monospace font for the id so depending on the id, the trace/span id could end up with different widths using min-content

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah now you say it I did notice that

`;

const StyledPanelHeader = styled(PanelHeader)<{align: 'left' | 'right'}>`
Expand All @@ -816,14 +861,6 @@ const EmptyStateText = styled('div')<{size: 'fontSizeExtraLarge' | 'fontSizeMedi
padding-bottom: ${space(1)};
`;

const Description = styled('div')`
${p => p.theme.overflowEllipsis};
display: flex;
flex-direction: row;
align-items: center;
gap: ${space(1)};
`;

const StyledPanelItem = styled(PanelItem)<{
align?: 'left' | 'center' | 'right';
overflow?: boolean;
Expand Down
142 changes: 134 additions & 8 deletions static/app/views/traces/fieldRenderers.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {useState} from 'react';
import {type Theme, useTheme} from '@emotion/react';
import {css, type Theme, useTheme} from '@emotion/react';
import styled from '@emotion/styled';
import type {Location} from 'history';

Expand All @@ -13,7 +13,7 @@ import PerformanceDuration from 'sentry/components/performanceDuration';
import TimeSince from 'sentry/components/timeSince';
import {Tooltip} from 'sentry/components/tooltip';
import {IconIssues} from 'sentry/icons';
import {t} from 'sentry/locale';
import {t, tn} from 'sentry/locale';
import {space} from 'sentry/styles/space';
import {generateLinkToEventInTraceView} from 'sentry/utils/discover/urls';
import {getShortEventId} from 'sentry/utils/events';
Expand All @@ -33,15 +33,20 @@ import type {SpanResult, TraceResult} from './content';
import type {Field} from './data';
import {getShortenedSdkName, getStylingSliceName} from './utils';

interface ProjectRendererProps {
projectSlug: string;
hideName?: boolean;
}
export const ProjectBadgeWrapper = styled('span')`
/**
* Max of 2 visible projects, 16px each, 2px border, 8px overlap.
*/
width: 32px;
min-width: 32px;
`;

export function SpanDescriptionRenderer({span}: {span: SpanResult<Field>}) {
return (
<Description>
<ProjectRenderer projectSlug={span.project} hideName />
<ProjectBadgeWrapper>
<ProjectRenderer projectSlug={span.project} hideName />
</ProjectBadgeWrapper>
<strong>{span['span.op']}</strong>
<em>{'\u2014'}</em>
<WrappingText>{span['span.description']}</WrappingText>
Expand All @@ -50,6 +55,126 @@ export function SpanDescriptionRenderer({span}: {span: SpanResult<Field>}) {
);
}

interface ProjectsRendererProps {
projectSlugs: string[];
maxVisibleProjects?: number;
}

export function ProjectsRenderer({
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was largely adapted from AvatarList however that component is very much specialized right now and doesn't extend to this use case very well. A good place to clean up in the future.

projectSlugs,
maxVisibleProjects = 2,
}: ProjectsRendererProps) {
const organization = useOrganization();

return (
<Projects orgId={organization.slug} slugs={projectSlugs}>
{({projects}) => {
const projectAvatars =
projects.length > 0 ? projects : projectSlugs.map(slug => ({slug}));
const numProjects = projectAvatars.length;
const numVisibleProjects =
maxVisibleProjects - numProjects >= 0 ? numProjects : maxVisibleProjects - 1;
const visibleProjectAvatars = projectAvatars
.slice(0, numVisibleProjects)
.reverse();
const collapsedProjectAvatars = projectAvatars.slice(numVisibleProjects);
const numCollapsedProjects = collapsedProjectAvatars.length;

return (
<ProjectList>
{numCollapsedProjects > 0 && (
<Tooltip
skipWrapper
title={
<CollapsedProjects>
{tn(
'This trace contains %s more project.',
'This trace contains %s more projects.',
numCollapsedProjects
)}
{collapsedProjectAvatars.map(project => (
<ProjectBadge
key={project.slug}
project={project}
avatarSize={16}
/>
))}
</CollapsedProjects>
}
>
<CollapsedBadge size={20} fontSize={10}>
+{numCollapsedProjects}
</CollapsedBadge>
</Tooltip>
)}
{visibleProjectAvatars.map(project => (
<StyledProjectBadge
key={project.slug}
hideName
project={project}
avatarSize={16}
avatarProps={{hasTooltip: true, tooltip: project.slug}}
/>
))}
</ProjectList>
);
}}
</Projects>
);
}

const ProjectList = styled('div')`
display: flex;
align-items: center;
flex-direction: row-reverse;
justify-content: flex-end;
padding-right: 8px;
`;

const CollapsedProjects = styled('div')`
width: 200px;
display: flex;
flex-direction: column;
gap: ${space(0.5)};
`;

const AvatarStyle = p => css`
border: 2px solid ${p.theme.background};
margin-right: -8px;
cursor: default;

&:hover {
z-index: 1;
}
`;

const StyledProjectBadge = styled(ProjectBadge)`
overflow: hidden;
z-index: 0;
${AvatarStyle}
`;

const CollapsedBadge = styled('div')<{fontSize: number; size: number}>`
display: flex;
align-items: center;
justify-content: center;
position: relative;
text-align: center;
font-weight: ${p => p.theme.fontWeightBold};
background-color: ${p => p.theme.gray200};
color: ${p => p.theme.gray300};
font-size: ${p => p.fontSize}px;
width: ${p => p.size}px;
height: ${p => p.size}px;
border-radius: ${p => p.theme.borderRadius};
${AvatarStyle}
`;

interface ProjectRendererProps {
projectSlug: string;
hideName?: boolean;
}

export function ProjectRenderer({projectSlug, hideName}: ProjectRendererProps) {
const organization = useOrganization();

Expand All @@ -62,6 +187,7 @@ export function ProjectRenderer({projectSlug, hideName}: ProjectRendererProps) {
hideName={hideName}
project={project ? project : {slug: projectSlug}}
avatarSize={16}
avatarProps={{hasTooltip: true, tooltip: projectSlug}}
/>
);
}}
Expand Down Expand Up @@ -464,7 +590,7 @@ const StyledTag = styled(Tag)`
cursor: ${p => (p.onClick ? 'pointer' : 'default')};
`;

const Description = styled('div')`
export const Description = styled('div')`
${p => p.theme.overflowEllipsis};
display: flex;
flex-direction: row;
Expand Down
Loading