feat(explore): Linking to spans in traceview from all tables (#78984)

There's no concept of `transaction.id` in eap. We now have access to `transaction.span_id` in both EAP and Indexed datasets. This PR enables sending `transaction.span_id` as `targetId` to the `events_trace` endpoint to ensure the transaction is always included in the response. Not the most proud of this solution, will follow up with PRs to completely replace `eventId`=`transaction.id` with `targetId`=`transaction.span_id`. PR also enables finding transactions by their span_ids in the trace view. With this change we can successfully scroll to spans and txns in the traceview from all explore tables using only span_ids. --------- Co-authored-by: Abdullah Khan <[email protected]>
getsentry · Oct 23, 2024 · 2a8874f · 2a8874f
1 parent e44845e
commit 2a8874f
Show file tree

Hide file tree

Showing 9 changed files with 29 additions and 24 deletions.
diff --git a/static/app/utils/discover/urls.tsx b/static/app/utils/discover/urls.tsx
@@ -52,11 +52,12 @@ export function generateLinkToEventInTraceView({
   eventId,
   transactionName,
   eventView,
+  targetId,
   demo,
   source,
   type = 'performance',
 }: {
-  eventId: string;
+  eventId: string | undefined;
   location: Location;
   organization: Organization;
   projectSlug: string;
@@ -67,6 +68,9 @@ export function generateLinkToEventInTraceView({
   isHomepage?: boolean;
   source?: string;
   spanId?: string;
+  // targetId represents the span id of the transaction. It will replace eventId once all links
+  // to trace view are updated to use spand ids of transactions instead of event ids.
+  targetId?: string;
   transactionName?: string;
   type?: 'performance' | 'discover';
 }) {
@@ -90,6 +94,7 @@ export function generateLinkToEventInTraceView({
       dateSelection,
       timestamp: normalizedTimestamp,
       eventId,
+      targetId,
       spanId,
       demo,
       location,

diff --git a/static/app/views/explore/tables/aggregatesTable.tsx b/static/app/views/explore/tables/aggregatesTable.tsx
@@ -149,7 +149,6 @@ export function AggregatesTable({}: AggregatesTableProps) {
                       )}
                       <FieldRenderer
                         column={columns[j]}
-                        dataset={dataset}
                         data={row}
                         unit={meta?.units?.[field]}
                         meta={meta}

diff --git a/static/app/views/explore/tables/fieldRenderer.spec.tsx b/static/app/views/explore/tables/fieldRenderer.spec.tsx
@@ -5,7 +5,6 @@ import {OrganizationFixture} from 'sentry-fixture/organization';
 import {render, screen} from 'sentry-test/reactTestingLibrary';
 
 import EventView from 'sentry/utils/discover/eventView';
-import {DiscoverDatasets} from 'sentry/utils/discover/types';
 
 import {FieldRenderer} from './fieldRenderer';
 
@@ -15,6 +14,7 @@ const mockedEventData = {
   trace: 'traceId',
   'span.op': 'test_op',
   'transaction.id': 'transactionId',
+  'transaction.span_id': 'transactionSpanId',
 };
 
 describe('FieldRenderer tests', function () {
@@ -46,7 +46,6 @@ describe('FieldRenderer tests', function () {
       <FieldRenderer
         column={eventView.getColumns()[3]}
         data={mockedEventData}
-        dataset={DiscoverDatasets.SPANS_INDEXED}
         meta={{}}
       />,
       {organization}
@@ -60,7 +59,6 @@ describe('FieldRenderer tests', function () {
       <FieldRenderer
         column={eventView.getColumns()[0]}
         data={mockedEventData}
-        dataset={DiscoverDatasets.SPANS_INDEXED}
         meta={{}}
       />,
       {organization}
@@ -69,7 +67,7 @@ describe('FieldRenderer tests', function () {
     expect(screen.getByText('spanId')).toBeInTheDocument();
     expect(screen.getByRole('link')).toHaveAttribute(
       'href',
-      `/organizations/org-slug/performance/trace/traceId/?eventId=transactionId&node=span-spanId&node=txn-transactionId&source=traces&statsPeriod=14d&timestamp=1727964900`
+      `/organizations/org-slug/performance/trace/traceId/?node=span-spanId&node=txn-transactionSpanId&source=traces&statsPeriod=14d&targetId=transactionSpanId&timestamp=1727964900`
     );
   });
 
@@ -78,7 +76,6 @@ describe('FieldRenderer tests', function () {
       <FieldRenderer
         column={eventView.getColumns()[4]}
         data={mockedEventData}
-        dataset={DiscoverDatasets.SPANS_INDEXED}
         meta={{}}
       />,
       {organization}
@@ -87,7 +84,7 @@ describe('FieldRenderer tests', function () {
     expect(screen.getByText('transactionId')).toBeInTheDocument();
     expect(screen.getByRole('link')).toHaveAttribute(
       'href',
-      `/organizations/org-slug/performance/trace/traceId/?eventId=transactionId&source=traces&statsPeriod=14d&timestamp=1727964900`
+      `/organizations/org-slug/performance/trace/traceId/?source=traces&statsPeriod=14d&targetId=transactionSpanId&timestamp=1727964900`
     );
   });
 
@@ -96,7 +93,6 @@ describe('FieldRenderer tests', function () {
       <FieldRenderer
         column={eventView.getColumns()[2]}
         data={mockedEventData}
-        dataset={DiscoverDatasets.SPANS_INDEXED}
         meta={{}}
       />,
       {organization}
@@ -114,7 +110,6 @@ describe('FieldRenderer tests', function () {
       <FieldRenderer
         column={eventView.getColumns()[1]}
         data={mockedEventData}
-        dataset={DiscoverDatasets.SPANS_INDEXED}
         meta={{}}
       />,
       {organization}

diff --git a/static/app/views/explore/tables/fieldRenderer.tsx b/static/app/views/explore/tables/fieldRenderer.tsx
@@ -6,7 +6,6 @@ import type {TableDataRow} from 'sentry/utils/discover/discoverQuery';
 import type {EventData, MetaType} from 'sentry/utils/discover/eventView';
 import EventView from 'sentry/utils/discover/eventView';
 import {getFieldRenderer} from 'sentry/utils/discover/fieldRenderers';
-import {DiscoverDatasets} from 'sentry/utils/discover/types';
 import {generateLinkToEventInTraceView} from 'sentry/utils/discover/urls';
 import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import {useLocation} from 'sentry/utils/useLocation';
@@ -22,12 +21,11 @@ import {useUserQuery} from '../hooks/useUserQuery';
 interface FieldProps {
   column: TableColumn<keyof TableDataRow>;
   data: EventData;
-  dataset: DiscoverDatasets;
   meta: MetaType;
   unit?: string;
 }
 
-export function FieldRenderer({data, dataset, meta, unit, column}: FieldProps) {
+export function FieldRenderer({data, meta, unit, column}: FieldProps) {
   const location = useLocation();
   const organization = useOrganization();
   const [userQuery, setUserQuery] = useUserQuery();
@@ -67,8 +65,8 @@ export function FieldRenderer({data, dataset, meta, unit, column}: FieldProps) {
       projectSlug: data.project,
       traceSlug: data.trace,
       timestamp: data.timestamp,
-      eventId:
-        dataset === DiscoverDatasets.SPANS_INDEXED ? data['transaction.id'] : undefined,
+      targetId: data['transaction.span_id'],
+      eventId: undefined,
       organization,
       location,
       spanId,

diff --git a/static/app/views/explore/tables/spansTable.tsx b/static/app/views/explore/tables/spansTable.tsx
@@ -43,7 +43,7 @@ export function SpansTable({}: SpansTableProps) {
       ...fields,
       'project',
       'trace',
-      'transaction.id',
+      'transaction.span_id',
       'span_id',
       'timestamp',
     ];
@@ -122,7 +122,6 @@ export function SpansTable({}: SpansTableProps) {
                     <TableBodyCell key={j}>
                       <FieldRenderer
                         column={columns[j]}
-                        dataset={dataset}
                         data={row}
                         unit={meta?.units?.[field]}
                         meta={meta}

diff --git a/static/app/views/performance/newTraceDetails/traceApi/useTrace.tsx b/static/app/views/performance/newTraceDetails/traceApi/useTrace.tsx
@@ -38,8 +38,8 @@ export function getTraceQueryParams(
   filters?: Partial<PageFilters>,
   options: {limit?: number; timestamp?: number} = {}
 ): {
-  eventId: string | undefined;
   limit: number;
+  targetId: string | undefined;
   timestamp: string | undefined;
   useSpans: number;
   demo?: string | undefined;
@@ -58,7 +58,7 @@ export function getTraceQueryParams(
     limit = parseInt(limit, 10);
   }
 
-  const eventId = decodeScalar(normalizedParams.eventId);
+  const targetId = decodeScalar(normalizedParams.targetId ?? normalizedParams.eventId);
 
   if (timestamp) {
     limit = limit ?? DEFAULT_TIMESTAMP_LIMIT;
@@ -83,7 +83,7 @@ export function getTraceQueryParams(
     demo,
     limit,
     timestamp: timestamp?.toString(),
-    eventId,
+    targetId,
     useSpans: 1,
   };
   for (const key in queryParams) {

diff --git a/static/app/views/performance/newTraceDetails/traceModels/traceTree.tsx b/static/app/views/performance/newTraceDetails/traceModels/traceTree.tsx
@@ -1113,7 +1113,9 @@ export class TraceTree extends TraceTreeEventDispatcher {
 
     return TraceTree.Find(start, node => {
       if (type === 'txn' && isTransactionNode(node)) {
-        return node.value.event_id === id;
+        // A transaction itself is a span and we are starting to treat it as such.
+        // Hence we check for both event_id and span_id.
+        return node.value.event_id === id || node.value.span_id === id;
       }
       if (type === 'span' && isSpanNode(node)) {
         return node.value.span_id === id;
@@ -1170,7 +1172,9 @@ export class TraceTree extends TraceTreeEventDispatcher {
   ): TraceTreeNode<TraceTree.NodeValue> | null {
     return TraceTree.Find(start, node => {
       if (isTransactionNode(node)) {
-        return node.value.event_id === eventId;
+        // A transaction itself is a span and we are starting to treat it as such.
+        // Hence we check for both event_id and span_id.
+        return node.value.event_id === eventId || node.value.span_id === eventId;
       }
       if (isSpanNode(node)) {
         return node.value.span_id === eventId;

diff --git a/static/app/views/performance/newTraceDetails/useTraceScrollToPath.tsx b/static/app/views/performance/newTraceDetails/useTraceScrollToPath.tsx
@@ -35,7 +35,7 @@ export function useTraceScrollToPath(
       const queryParams = qs.parse(location.search);
 
       scrollToNode = {
-        eventId: queryParams.eventId as string | undefined,
+        eventId: (queryParams.eventId ?? queryParams.targetId) as string | undefined,
         path: decodeScrollQueue(queryParams.node) as TraceTree.NodePath[] | undefined,
       };
     }

diff --git a/static/app/views/performance/traceDetails/utils.tsx b/static/app/views/performance/traceDetails/utils.tsx
@@ -23,6 +23,7 @@ export function getTraceDetailsUrl({
   timestamp,
   spanId,
   eventId,
+  targetId,
   demo,
   location,
   source,
@@ -35,6 +36,9 @@ export function getTraceDetailsUrl({
   eventId?: string;
   source?: string;
   spanId?: string;
+  // targetId represents the span id of the transaction. It will replace eventId once all links
+  // to trace view are updated to use spand ids of transactions instead of event ids.
+  targetId?: string;
   timestamp?: string | number;
 }): LocationDescriptorObject {
   const {start, end, statsPeriod} = dateSelection;
@@ -59,7 +63,7 @@ export function getTraceDetailsUrl({
 
   if (organization.features.includes('trace-view-v1')) {
     if (spanId) {
-      queryParams.node = [`span-${spanId}`, `txn-${eventId}`];
+      queryParams.node = [`span-${spanId}`, `txn-${targetId ?? eventId}`];
     }
     return {
       pathname: normalizeUrl(
@@ -69,6 +73,7 @@ export function getTraceDetailsUrl({
         ...queryParams,
         timestamp: getTimeStampFromTableDateField(timestamp),
         eventId,
+        targetId,
         demo,
         source,
       },