-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
feat(dashboards): always link to transaction summary from transaction field #106116
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.
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
feat(dashboards): always link to transaction summary from transaction field #106116
Conversation
Skip wrapping transaction field in TransactionLink for SPANS widgets since their customRenderer already returns a link component. This prevents invalid HTML with nested anchor tags that causes React 19 hydration errors. Fixes test: "links to the spans page when View span samples is clicked"
| if (data[SpanFields.SPAN_OP]) { | ||
| filters.addFilterValue('transaction.op', data[SpanFields.SPAN_OP]); | ||
| } | ||
| if (data[SpanFields.REQUEST_METHOD]) { | ||
| filters.addFilterValue('http.method', data[SpanFields.REQUEST_METHOD]); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: In transactionSummaryRouteWithQuery, spreading additionalQuery overwrites the existing query parameter, causing original dashboard filters to be discarded when clicking a transaction link.
Severity: HIGH
🔍 Detailed Analysis
The renderTransactionAsLinkable function calls transactionSummaryRouteWithQuery, passing new filters in an additionalQuery object with a query key. Inside transactionSummaryRouteWithQuery, the original dashboard filters are processed and assigned to a query property. However, the ...additionalQuery spread operator is used afterwards, which overwrites the original query property with the new filters. This results in the user losing all their existing dashboard filter context when they click the generated transaction link, as only the newly added filters are preserved in the destination URL.
💡 Suggested Fix
The filters from additionalQuery should be merged with the existing searchFilter instead of overwriting it. Combine the new filters from filters.formatString() with the searchFilter from the original query before assigning the result to the final query property. This will preserve the user's dashboard context.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: static/app/views/dashboards/datasetConfig/spans.tsx#L517-L522
Potential issue: The `renderTransactionAsLinkable` function calls
`transactionSummaryRouteWithQuery`, passing new filters in an `additionalQuery` object
with a `query` key. Inside `transactionSummaryRouteWithQuery`, the original dashboard
filters are processed and assigned to a `query` property. However, the
`...additionalQuery` spread operator is used afterwards, which overwrites the original
`query` property with the new filters. This results in the user losing all their
existing dashboard filter context when they click the generated transaction link, as
only the newly added filters are preserved in the destination URL.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 8548046
| project => | ||
| project.slug && [data['project.name'], data.project].includes(project.slug) | ||
| ); | ||
| projectID = projectMatch ? [projectMatch.id] : undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing projects in baggage breaks project matching
Medium Severity
The renderTransactionAsLinkable function expects projects to be available in the baggage parameter for project-matching logic, but the makeBaggage function in widgetViewerModal.tsx does not include projects in the returned object. Since projects will always be undefined in the baggage, the project-matching fallback at lines 507-511 will never work. Transaction summary links will only have a project filter when location.query.project is a single string, otherwise projectID will be undefined.
Additional Locations (1)
| projectID, | ||
| query: location?.query, | ||
| additionalQuery: {query: filters.formatString()}, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing project filter fallback for multi-project selection
Low Severity
The new renderTransactionAsLinkable function is missing a fallback that exists in getTargetForTransactionSummaryLink. When projectID cannot be determined but filterProjects exists (e.g., when multiple projects are selected), the existing function applies target.query.project = filterProjects as a fallback. This fallback is absent in the new function, so when multiple projects are selected and the project cannot be matched from row data, the generated link will have no project filter instead of preserving the original filter selection.
Updates the transaction field so it always links to the transaction summary in the spans dataset, additionally the project, span op and request method fields are added as filters when available in the table.

This is to support the backend overview transactions table.