From 82601d9dfbf08f33a81f7a1cace71c720101092e Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Mon, 29 Jul 2024 12:17:42 -0400 Subject: [PATCH 1/3] feat(profiling): Support both profile formats in transaction samples tab Similar to #75099. This will render a profiling icon to link to the transaction/continuous profile automatically based on the data ingested. --- .../transactionEvents/content.tsx | 145 ++++++++++++------ .../transactionEvents/eventsTable.tsx | 37 +++-- 2 files changed, 125 insertions(+), 57 deletions(-) diff --git a/static/app/views/performance/transactionSummary/transactionEvents/content.tsx b/static/app/views/performance/transactionSummary/transactionEvents/content.tsx index 2e163c909e344d..6c7b8094bcff8f 100644 --- a/static/app/views/performance/transactionSummary/transactionEvents/content.tsx +++ b/static/app/views/performance/transactionSummary/transactionEvents/content.tsx @@ -1,3 +1,4 @@ +import {useMemo} from 'react'; import styled from '@emotion/styled'; import type {Location} from 'history'; import omit from 'lodash/omit'; @@ -17,6 +18,10 @@ import type {Project} from 'sentry/types/project'; import {trackAnalytics} from 'sentry/utils/analytics'; import {browserHistory} from 'sentry/utils/browserHistory'; import type EventView from 'sentry/utils/discover/eventView'; +import { + isSpanOperationBreakdownField, + SPAN_OP_RELATIVE_BREAKDOWN_FIELD, +} from 'sentry/utils/discover/fields'; import type {WebVital} from 'sentry/utils/fields'; import {decodeScalar} from 'sentry/utils/queryString'; import projectSupportsReplay from 'sentry/utils/replays/projectSupportsReplay'; @@ -72,57 +77,110 @@ function EventsContent(props: Props) { projects, } = props; const routes = useRoutes(); - const eventView = originalEventView.clone(); - const transactionsListTitles = TRANSACTIONS_LIST_TITLES.slice(); - const project = projects.find(p => p.id === projectId); - const fields = [...eventView.fields]; + const {eventView, titles, shouldRenderColumn} = useMemo(() => { + const eventViewClone = originalEventView.clone(); + const transactionsListTitles = TRANSACTIONS_LIST_TITLES.slice(); + const project = projects.find(p => p.id === projectId); - if (webVital) { - transactionsListTitles.splice(3, 0, webVital); - } + const fields = [...eventViewClone.fields]; - const spanOperationBreakdownConditions = filterToSearchConditions( - spanOperationBreakdownFilter, - location - ); + if (webVital) { + transactionsListTitles.splice(3, 0, webVital); + } - if (spanOperationBreakdownConditions) { - eventView.query = `${eventView.query} ${spanOperationBreakdownConditions}`.trim(); - transactionsListTitles.splice(2, 1, t('%s duration', spanOperationBreakdownFilter)); - } + const spanOperationBreakdownConditions = filterToSearchConditions( + spanOperationBreakdownFilter, + location + ); - const platform = platformToPerformanceType(projects, eventView.project); - if (platform === ProjectPerformanceType.BACKEND) { - const userIndex = transactionsListTitles.indexOf('user'); - if (userIndex > 0) { - transactionsListTitles.splice(userIndex + 1, 0, 'http.method'); - fields.splice(userIndex + 1, 0, {field: 'http.method'}); + if (spanOperationBreakdownConditions) { + eventViewClone.query = + `${eventViewClone.query} ${spanOperationBreakdownConditions}`.trim(); + transactionsListTitles.splice(2, 1, t('%s duration', spanOperationBreakdownFilter)); } - } - if ( - organization.features.includes('profiling') && - project && - // only show for projects that already sent a profile - // once we have a more compact design we will show this for - // projects that support profiling as well - project.hasProfiles - ) { - transactionsListTitles.push(t('profile')); - fields.push({field: 'profile.id'}); - } + const platform = platformToPerformanceType(projects, eventViewClone.project); + if (platform === ProjectPerformanceType.BACKEND) { + const userIndex = transactionsListTitles.indexOf('user'); + if (userIndex > 0) { + transactionsListTitles.splice(userIndex + 1, 0, 'http.method'); + fields.splice(userIndex + 1, 0, {field: 'http.method'}); + } + } - if ( - organization.features.includes('session-replay') && - project && - projectSupportsReplay(project) - ) { - transactionsListTitles.push(t('replay')); - fields.push({field: 'replayId'}); - } + let hasProfilerFields = false; + if ( + // only show for projects that already sent a profile + // once we have a more compact design we will show this for + // projects that support profiling as well + project?.hasProfiles && + (organization.features.includes('profiling') || + organization.features.includes('continuous-profiling')) + ) { + transactionsListTitles.push(t('profile')); + + if (organization.features.includes('profiling')) { + fields.push({field: 'profile.id'}); + } + + if (organization.features.includes('continuous-profiling')) { + hasProfilerFields = true; + fields.push({field: 'profiler.id'}); + fields.push({field: 'thread.id'}); + fields.push({field: 'precise.start_ts'}); + fields.push({field: 'precise.finish_ts'}); + } + } - eventView.fields = fields; + if ( + organization.features.includes('session-replay') && + project && + projectSupportsReplay(project) + ) { + transactionsListTitles.push(t('replay')); + fields.push({field: 'replayId'}); + } + + eventViewClone.fields = fields; + + const containsSpanOpsBreakdown = fields.some( + ({field}) => field === SPAN_OP_RELATIVE_BREAKDOWN_FIELD + ); + + const shouldRenderColumnFn = (col: string): boolean => { + if (containsSpanOpsBreakdown && isSpanOperationBreakdownField(col)) { + return false; + } + + if (hasProfilerFields) { + if ( + col === 'profiler.id' || + col === 'thread.id' || + col === 'precise.start_ts' || + col === 'precise.finish_ts' + ) { + return false; + } + } + + return true; + }; + + return { + eventView: eventViewClone, + titles: transactionsListTitles, + shouldRenderColumn: shouldRenderColumnFn, + }; + }, [ + originalEventView, + location, + organization, + projects, + projectId, + spanOperationBreakdownFilter, + webVital, + ]); return ( @@ -133,8 +191,9 @@ function EventsContent(props: Props) { routes={routes} location={location} setError={setError} - columnTitles={transactionsListTitles} + columnTitles={titles} transactionName={transactionName} + shouldRenderColumn={shouldRenderColumn} /> ); diff --git a/static/app/views/performance/transactionSummary/transactionEvents/eventsTable.tsx b/static/app/views/performance/transactionSummary/transactionEvents/eventsTable.tsx index 0f738ea100f6b3..59947762c6b817 100644 --- a/static/app/views/performance/transactionSummary/transactionEvents/eventsTable.tsx +++ b/static/app/views/performance/transactionSummary/transactionEvents/eventsTable.tsx @@ -5,6 +5,7 @@ import type {Location, LocationDescriptor, LocationDescriptorObject} from 'histo import groupBy from 'lodash/groupBy'; import {Client} from 'sentry/api'; +import {LinkButton} from 'sentry/components/button'; import type {GridColumn} from 'sentry/components/gridEditable'; import GridEditable, {COL_WIDTH_UNDEFINED} from 'sentry/components/gridEditable'; import SortLink from 'sentry/components/gridEditable/sortLink'; @@ -12,6 +13,7 @@ import Link from 'sentry/components/links/link'; import Pagination from 'sentry/components/pagination'; import QuestionTooltip from 'sentry/components/questionTooltip'; import {Tooltip} from 'sentry/components/tooltip'; +import {IconProfiling} from 'sentry/icons'; import {t, tct} from 'sentry/locale'; import type {IssueAttachment, Organization} from 'sentry/types'; import {trackAnalytics} from 'sentry/utils/analytics'; @@ -24,11 +26,11 @@ import {getFieldRenderer} from 'sentry/utils/discover/fieldRenderers'; import { fieldAlignment, getAggregateAlias, - isSpanOperationBreakdownField, SPAN_OP_RELATIVE_BREAKDOWN_FIELD, } from 'sentry/utils/discover/fields'; import {generateLinkToEventInTraceView} from 'sentry/utils/discover/urls'; import ViewReplayLink from 'sentry/utils/discover/viewReplayLink'; +import {isEmptyObject} from 'sentry/utils/object/isEmptyObject'; import parseLinkHeader from 'sentry/utils/parseLinkHeader'; import {VisuallyCompleteWithData} from 'sentry/utils/performanceForSentry'; import CellAction, {Actions, updateQuery} from 'sentry/views/discover/table/cellAction'; @@ -68,6 +70,7 @@ type Props = { organization: Organization; routes: RouteContextInterface['routes']; setError: (msg: string | undefined) => void; + shouldRenderColumn: (col: string) => boolean; transactionName: string; columnTitles?: string[]; customColumns?: ('attachments' | 'minidump')[]; @@ -242,7 +245,15 @@ class EventsTable extends Component { handleCellAction={this.handleCellAction(column)} allowActions={allowActions} > - {target ? {rendered} : rendered} +
+ + + +
); @@ -371,27 +382,25 @@ class EventsTable extends Component { }; render() { - const {eventView, organization, location, setError, referrer, isEventLoading} = - this.props; + const { + eventView, + organization, + location, + setError, + referrer, + isEventLoading, + shouldRenderColumn, + } = this.props; const totalEventsView = eventView.clone(); totalEventsView.sorts = []; totalEventsView.fields = [{field: 'count()', width: -1}]; const {widths} = this.state; - const containsSpanOpsBreakdown = eventView - .getColumns() - .find( - (col: TableColumn) => - col.name === SPAN_OP_RELATIVE_BREAKDOWN_FIELD - ); const columnOrder = eventView .getColumns() - .filter( - (col: TableColumn) => - !containsSpanOpsBreakdown || !isSpanOperationBreakdownField(col.name) - ) + .filter((col: TableColumn) => shouldRenderColumn(col.name)) .map((col: TableColumn, i: number) => { if (typeof widths[i] === 'number') { return {...col, width: widths[i]}; From 57ca608c2aed465db3fb518e5f81b5ab01cd2373 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Mon, 29 Jul 2024 12:40:14 -0400 Subject: [PATCH 2/3] simplify render condition --- .../transactionEvents/content.tsx | 33 +-------------- .../transactionEvents/eventsTable.tsx | 40 ++++++++++++++----- 2 files changed, 30 insertions(+), 43 deletions(-) diff --git a/static/app/views/performance/transactionSummary/transactionEvents/content.tsx b/static/app/views/performance/transactionSummary/transactionEvents/content.tsx index 6c7b8094bcff8f..3ba03379c9bfbf 100644 --- a/static/app/views/performance/transactionSummary/transactionEvents/content.tsx +++ b/static/app/views/performance/transactionSummary/transactionEvents/content.tsx @@ -18,10 +18,6 @@ import type {Project} from 'sentry/types/project'; import {trackAnalytics} from 'sentry/utils/analytics'; import {browserHistory} from 'sentry/utils/browserHistory'; import type EventView from 'sentry/utils/discover/eventView'; -import { - isSpanOperationBreakdownField, - SPAN_OP_RELATIVE_BREAKDOWN_FIELD, -} from 'sentry/utils/discover/fields'; import type {WebVital} from 'sentry/utils/fields'; import {decodeScalar} from 'sentry/utils/queryString'; import projectSupportsReplay from 'sentry/utils/replays/projectSupportsReplay'; @@ -78,7 +74,7 @@ function EventsContent(props: Props) { } = props; const routes = useRoutes(); - const {eventView, titles, shouldRenderColumn} = useMemo(() => { + const {eventView, titles} = useMemo(() => { const eventViewClone = originalEventView.clone(); const transactionsListTitles = TRANSACTIONS_LIST_TITLES.slice(); const project = projects.find(p => p.id === projectId); @@ -109,7 +105,6 @@ function EventsContent(props: Props) { } } - let hasProfilerFields = false; if ( // only show for projects that already sent a profile // once we have a more compact design we will show this for @@ -125,7 +120,6 @@ function EventsContent(props: Props) { } if (organization.features.includes('continuous-profiling')) { - hasProfilerFields = true; fields.push({field: 'profiler.id'}); fields.push({field: 'thread.id'}); fields.push({field: 'precise.start_ts'}); @@ -144,33 +138,9 @@ function EventsContent(props: Props) { eventViewClone.fields = fields; - const containsSpanOpsBreakdown = fields.some( - ({field}) => field === SPAN_OP_RELATIVE_BREAKDOWN_FIELD - ); - - const shouldRenderColumnFn = (col: string): boolean => { - if (containsSpanOpsBreakdown && isSpanOperationBreakdownField(col)) { - return false; - } - - if (hasProfilerFields) { - if ( - col === 'profiler.id' || - col === 'thread.id' || - col === 'precise.start_ts' || - col === 'precise.finish_ts' - ) { - return false; - } - } - - return true; - }; - return { eventView: eventViewClone, titles: transactionsListTitles, - shouldRenderColumn: shouldRenderColumnFn, }; }, [ originalEventView, @@ -193,7 +163,6 @@ function EventsContent(props: Props) { setError={setError} columnTitles={titles} transactionName={transactionName} - shouldRenderColumn={shouldRenderColumn} /> ); diff --git a/static/app/views/performance/transactionSummary/transactionEvents/eventsTable.tsx b/static/app/views/performance/transactionSummary/transactionEvents/eventsTable.tsx index 59947762c6b817..16c18cebb3a915 100644 --- a/static/app/views/performance/transactionSummary/transactionEvents/eventsTable.tsx +++ b/static/app/views/performance/transactionSummary/transactionEvents/eventsTable.tsx @@ -26,6 +26,7 @@ import {getFieldRenderer} from 'sentry/utils/discover/fieldRenderers'; import { fieldAlignment, getAggregateAlias, + isSpanOperationBreakdownField, SPAN_OP_RELATIVE_BREAKDOWN_FIELD, } from 'sentry/utils/discover/fields'; import {generateLinkToEventInTraceView} from 'sentry/utils/discover/urls'; @@ -49,6 +50,23 @@ import { import type {TitleProps} from './operationSort'; import OperationSort from './operationSort'; +function shouldRenderColumn(containsSpanOpsBreakdown: boolean, col: string): boolean { + if (containsSpanOpsBreakdown && isSpanOperationBreakdownField(col)) { + return false; + } + + if ( + col === 'profiler.id' || + col === 'thread.id' || + col === 'precise.start_ts' || + col === 'precise.finish_ts' + ) { + return false; + } + + return true; +} + function OperationTitle({onClick}: TitleProps) { return (
@@ -70,7 +88,6 @@ type Props = { organization: Organization; routes: RouteContextInterface['routes']; setError: (msg: string | undefined) => void; - shouldRenderColumn: (col: string) => boolean; transactionName: string; columnTitles?: string[]; customColumns?: ('attachments' | 'minidump')[]; @@ -382,25 +399,26 @@ class EventsTable extends Component { }; render() { - const { - eventView, - organization, - location, - setError, - referrer, - isEventLoading, - shouldRenderColumn, - } = this.props; + const {eventView, organization, location, setError, referrer, isEventLoading} = + this.props; const totalEventsView = eventView.clone(); totalEventsView.sorts = []; totalEventsView.fields = [{field: 'count()', width: -1}]; const {widths} = this.state; + const containsSpanOpsBreakdown = eventView + .getColumns() + .find( + (col: TableColumn) => + col.name === SPAN_OP_RELATIVE_BREAKDOWN_FIELD + ); const columnOrder = eventView .getColumns() - .filter((col: TableColumn) => shouldRenderColumn(col.name)) + .filter((col: TableColumn) => + shouldRenderColumn(containsSpanOpsBreakdown, col.name) + ) .map((col: TableColumn, i: number) => { if (typeof widths[i] === 'number') { return {...col, width: widths[i]}; From 976ad975fcf4315674a199f92b38c2cc1aa08421 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Mon, 29 Jul 2024 12:45:13 -0400 Subject: [PATCH 3/3] fix typing --- .../transactionSummary/transactionEvents/eventsTable.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/app/views/performance/transactionSummary/transactionEvents/eventsTable.tsx b/static/app/views/performance/transactionSummary/transactionEvents/eventsTable.tsx index 16c18cebb3a915..f96b02f86f575e 100644 --- a/static/app/views/performance/transactionSummary/transactionEvents/eventsTable.tsx +++ b/static/app/views/performance/transactionSummary/transactionEvents/eventsTable.tsx @@ -407,7 +407,7 @@ class EventsTable extends Component { totalEventsView.fields = [{field: 'count()', width: -1}]; const {widths} = this.state; - const containsSpanOpsBreakdown = eventView + const containsSpanOpsBreakdown = !!eventView .getColumns() .find( (col: TableColumn) =>