Skip to content

[No QA] [Sentry] Stop starting ManualOpenReport for non-OpenReport actions #84595

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.

Sign up for GitHub

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

Merged
rlinoz merged 1 commit into from
Mar 9, 2026
Merged

[No QA] [Sentry] Stop starting ManualOpenReport for non-OpenReport actions #84595

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 8 additions & 12 deletions src/libs/Navigation/Navigation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,21 +298,17 @@ function navigate(route: Route, options?: LinkToOptions) {
return;
}

// Start a Sentry span for report navigation
if (route.startsWith('r/') || route.startsWith('search/r/') || route.startsWith('e/')) {
const routePath = Str.cutAfter(route, '?');
const reportIDMatch = route.match(/^(?:search\/)?(?:r|e)\/(\w+)/);
const reportID = reportIDMatch?.at(1);
// Start a Sentry span for report navigation — only for exact report-open routes, not sub-pages.
// Matches: r/<id>, search/r/<id>, search/view/<id>, e/<id>
const reportOpenMatch = Str.cutAfter(route, '?').match(/^(search\/(?:r|view)|r|e)\/(\w+)$/);
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include report-action routes in report-open matcher

The new regex only matches r/<reportID> and search/view|r/<reportID> exactly, so valid open-report URLs that include an action ID (for example r/<reportID>/<reportActionID> and search/view/<reportID>/<reportActionID>) no longer start ManualOpenReport spans. Those routes are still first-class report opens (ROUTES.REPORT_WITH_ID.route and ROUTES.SEARCH_REPORT.route both define optional :reportActionID), so this change drops telemetry for deep links/parent-action navigation paths and can skew open-report performance data.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

@rlinoz rlinoz Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is very infrequent and fine for now, we might even want to have a different metric for deeplinking into a report.

if (reportOpenMatch) {
const routePrefix = reportOpenMatch.at(1);
const reportID = reportOpenMatch.at(2);
if (reportID) {
const spanId = `${CONST.TELEMETRY.SPAN_OPEN_REPORT}_${reportID}`;
let span = getSpan(spanId);
if (!span) {
let spanName = '/r/*';
if (route.startsWith('search/r/')) {
spanName = '/search/r/*';
} else if (route.startsWith('e/')) {
spanName = '/e/*';
}
const spanName = `/${routePrefix}/*`;
span = startSpan(spanId, {
name: spanName,
op: CONST.TELEMETRY.SPAN_OPEN_REPORT,
Expand All @@ -321,7 +317,7 @@ function navigate(route: Route, options?: LinkToOptions) {
span.setAttributes({
[CONST.TELEMETRY.ATTRIBUTE_REPORT_ID]: reportID,
[CONST.TELEMETRY.ATTRIBUTE_ROUTE_FROM]: getActiveRouteWithoutParams(),
[CONST.TELEMETRY.ATTRIBUTE_ROUTE_TO]: Str.cutAfter(routePath, '?'),
[CONST.TELEMETRY.ATTRIBUTE_ROUTE_TO]: Str.cutAfter(route, '?'),
});
}
}
Expand Down
Loading