feat(trace-view): Add UI fallback to old view for STs #73684
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| import type {Location, LocationDescriptorObject} from 'history'; | ||
|
|
||
| import {PAGE_URL_PARAM} from 'sentry/constants/pageFilters'; | ||
| import type {Organization} from 'sentry/types'; | ||
| import {getTimeStampFromTableDateField} from 'sentry/utils/dates'; | ||
| import type { | ||
| EventLite, | ||
|
|
@@ -29,7 +29,7 @@ export function getTraceDetailsUrl({ | |
| }: { | ||
| dateSelection; | ||
| location: Location; | ||
| organization: Organization; | ||
| traceSlug: string; | ||
| demo?: string; | ||
| eventId?: string; | ||
|
|
@@ -46,6 +46,17 @@ export function getTraceDetailsUrl({ | |
| [PAGE_URL_PARAM.PAGE_END]: end, | ||
| }; | ||
|
|
||
| const oldTraceUrl = { | ||
| pathname: normalizeUrl( | ||
| `/organizations/${organization.slug}/performance/trace/${traceSlug}/` | ||
| ), | ||
| query: queryParams, | ||
| }; | ||
|
|
||
| if (shouldForceRouteToOldView(organization, timestamp)) { | ||
| return oldTraceUrl; | ||
| } | ||
|
|
||
| if (organization.features.includes('trace-view-v1')) { | ||
| if (spanId) { | ||
| queryParams.node = [`span-${spanId}`, `txn-${eventId}`]; | ||
|
|
@@ -68,12 +79,28 @@ export function getTraceDetailsUrl({ | |
| queryParams.limit = DEFAULT_TRACE_ROWS_LIMIT; | ||
| } | ||
|
|
||
| return oldTraceUrl; | ||
| } | ||
|
|
||
| /** | ||
| * Single tenant, on-premise etc. users may not have span extraction enabled. | ||
| * | ||
| * This code can be removed at the time we're sure all STs have rolled out span extraction. | ||
| */ | ||
| export function shouldForceRouteToOldView( | ||
| organization: Organization, | ||
| timestamp: string | number | undefined | ||
| ) { | ||
| const usableTimestamp = getTimeStampFromTableDateField(timestamp); | ||
| if (!usableTimestamp) { | ||
| // Timestamps must always be provided for the new view, if it doesn't exist, fall back to the old view. | ||
|
There was a problem hiding this comment. @k-fish , it just helps make the query faster. The traceview still loads without it. There was a problem hiding this comment. I realize it works on the api but the frontend it should be mandatory no? it significantly improves trace performance, I had assumed all callsites were sending it where appropriate for the new view. There was a problem hiding this comment. Yeah we do it from all callsites. However, even without it the new traceview should load faster than the old traceview I believe. But yeah, just wanted to point it out in case you thought that the new view wouldn't work at all without timestamp. |
||
| return true; | ||
| } | ||
|
|
||
| return ( | ||
| organization.extraOptions?.traces.checkSpanExtractionDate && | ||
| organization.extraOptions?.traces.spansExtractionDate <= usableTimestamp | ||
| ); | ||
| } | ||
|
|
||
| function transactionVisitor() { | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
The new line break here makes the comment's location misleading IMO.
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.
Deprecated should actually be annotated, which makes it show up in editors and also moves with the key properly, going to change it to that instead.