feat(trace-view): Add UI fallback to old view for STs #73684
Conversation
Singletenant (and on-prem etc.) may not be fully rolled out for span extraction. This means the new trace-view which is entirely span based won't work. This causes links to fallback to the old trace view if the organization is one without span-extraction rolled out, then proceeds to check the rollout date so we can offer the new experience where possible right away.
github-actions
bot
added
the
Scope: Frontend
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #73684 +/- ##
==========================================
+ Coverage 78.07% 78.11% +0.04%
==========================================
Files 6649 6664 +15
Lines 297218 297762 +544
Branches 51148 51214 +66
==========================================
+ Hits 232062 232610 +548
- Misses 58839 58864 +25
+ Partials 6317 6288 -29
|
static/app/types/organization.tsx
Outdated
| @@ -48,7 +48,8 @@ export interface Organization extends OrganizationSummary { | |||
| allowJoinRequests: boolean; | |||
| allowSharedIssues: boolean; | |||
| attachmentsRole: string; | |||
| availableRoles: {id: string; name: string}[]; // Deprecated, use orgRoleList | |||
| availableRoles: {id: string; name: string}[]; | |||
| // Deprecated, use orgRoleList | |||
There was a problem hiding this comment.
The new line break here makes the comment's location misleading IMO.
There was a problem hiding this comment.
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.
| ) { | ||
| 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.
|
@Abdkhan14 are there any other call sites / a better way instead of a redirect to do this? |
@k-fish, you've covered all entry points. We need the traceSlug from the fetched event so re-routing seems to be a straightforward solution here. |
Summary
Singletenant (and on-prem etc.) may not be fully rolled out for span extraction. This means the new trace-view which is entirely span based won't work. This causes links to fallback to the old trace view if the organization is one without span-extraction rolled out, then proceeds to check the rollout date so we can offer the new experience where possible right away.
Aside
OrganizationSummarydidn't need to be narrowed, so switched it out for the fullOrganizationprovided at the top level byuseOrganization, can't see a reason why we wouldn't.