Skip to content

[No QA] Fix ManualSendMessage metrics measurements #82321

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

Open
cristipaval wants to merge 10 commits into
base: main
Choose a base branch
from
+104 −34
Open

[No QA] Fix ManualSendMessage metrics measurements #82321

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e049759
Pass optimistic reportActionID to onSubmit
cristipaval Feb 12, 2026
8b06a71
Measure only when the chat is scrolled to the bottom
cristipaval Feb 12, 2026
447467a
Add reportActionID to the spanID
cristipaval Feb 12, 2026
e79e241
Cleanup orphaned spans
cristipaval Feb 12, 2026
87b4c97
Prettier
cristipaval Feb 12, 2026
506c5a5
Refactor the ActionListContext
cristipaval Feb 13, 2026
f756382
send-message span into a separate useEffect
cristipaval Feb 13, 2026
567daab
Merge remote-tracking branch 'origin/main' into fix-ManualSendMessage…
cristipaval Feb 13, 2026
b598864
Fix ts and lint checks
cristipaval Feb 13, 2026
9f5a0fd
Cleanup spans in SearchMoneyRequestReportPage
cristipaval Feb 13, 2026
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
32 changes: 28 additions & 4 deletions src/libs/actions/Report/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@
shouldPlaySound?: boolean;
isInSidePanel?: boolean;
pregeneratedResponseParams?: PregeneratedResponseParams;
reportActionID?: string;
};

type AddActionsParams = {
Expand All @@ -292,6 +293,7 @@
file?: FileObject;
isInSidePanel?: boolean;
pregeneratedResponseParams?: PregeneratedResponseParams;
reportActionID?: string;
};

type AddAttachmentWithCommentParams = {
Expand All @@ -312,7 +314,7 @@
/** @deprecated This value is deprecated and will be removed soon after migration. Use the email from useCurrentUserPersonalDetails hook instead. */
let deprecatedCurrentUserLogin: string | undefined;

Onyx.connect({

Check warning on line 317 in src/libs/actions/Report/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function

Check warning on line 317 in src/libs/actions/Report/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.SESSION,
callback: (value) => {
// When signed out, val is undefined
Expand All @@ -326,7 +328,7 @@
},
});

Onyx.connect({

Check warning on line 331 in src/libs/actions/Report/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function

Check warning on line 331 in src/libs/actions/Report/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.CONCIERGE_REPORT_ID,
callback: (value) => (conciergeReportIDOnyxConnect = value),
});
Expand All @@ -334,7 +336,7 @@
// map of reportID to all reportActions for that report
const allReportActions: OnyxCollection<ReportActions> = {};

Onyx.connect({

Check warning on line 339 in src/libs/actions/Report/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function

Check warning on line 339 in src/libs/actions/Report/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,
callback: (actions, key) => {
if (!key || !actions) {
Expand All @@ -346,7 +348,7 @@
});

let allReports: OnyxCollection<Report>;
Onyx.connect({

Check warning on line 351 in src/libs/actions/Report/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -355,7 +357,7 @@
});

let allPersonalDetails: OnyxEntry<PersonalDetailsList> = {};
Onyx.connect({

Check warning on line 360 in src/libs/actions/Report/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
callback: (value) => {
allPersonalDetails = value ?? {};
Expand All @@ -370,7 +372,7 @@
});

let onboarding: OnyxEntry<Onboarding>;
Onyx.connect({

Check warning on line 375 in src/libs/actions/Report/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.NVP_ONBOARDING,
callback: (val) => {
if (Array.isArray(val)) {
Expand All @@ -381,7 +383,7 @@
});

let deprecatedIntroSelected: OnyxEntry<IntroSelected> = {};
Onyx.connect({

Check warning on line 386 in src/libs/actions/Report/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.NVP_INTRO_SELECTED,
callback: (val) => (deprecatedIntroSelected = val),
});
Expand Down Expand Up @@ -609,7 +611,18 @@
* @param isInSidePanel - Whether the comment is being added from the side panel
* @param pregeneratedResponseParams - Optional params for pre-generated response (API only, no optimistic action - used when response display is delayed)
*/
function addActions({report, notifyReportID, ancestors, timezoneParam, currentUserAccountID, text = '', file, isInSidePanel = false, pregeneratedResponseParams}: AddActionsParams) {
function addActions({
report,
notifyReportID,
ancestors,
timezoneParam,
currentUserAccountID,
text = '',
file,
isInSidePanel = false,
pregeneratedResponseParams,
reportActionID,
Copy link
Contributor

@roryabraham roryabraham Feb 13, 2026

Choose a reason for hiding this comment

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

question: Would an alternative here be to return the reportActionID for the newly created actions? that way it doesn't have to be passed in. NAB

}: AddActionsParams) {
if (!report?.reportID) {
return;
}
Expand All @@ -620,7 +633,7 @@
let commandName: typeof WRITE_COMMANDS.ADD_COMMENT | typeof WRITE_COMMANDS.ADD_ATTACHMENT | typeof WRITE_COMMANDS.ADD_TEXT_AND_ATTACHMENT = WRITE_COMMANDS.ADD_COMMENT;

if (text && !file) {
const reportComment = buildOptimisticAddCommentReportAction(text, undefined, undefined, undefined, reportID);
const reportComment = buildOptimisticAddCommentReportAction(text, undefined, undefined, undefined, reportID, reportActionID);
reportCommentAction = reportComment.reportAction;
reportCommentText = reportComment.commentText;
}
Expand Down Expand Up @@ -848,11 +861,22 @@
}

/** Add a single comment to a report */
function addComment({report, notifyReportID, ancestors, text, timezoneParam, currentUserAccountID, shouldPlaySound, isInSidePanel, pregeneratedResponseParams}: AddCommentParams) {
function addComment({
report,
notifyReportID,
ancestors,
text,
timezoneParam,
currentUserAccountID,
shouldPlaySound,
isInSidePanel,
pregeneratedResponseParams,
reportActionID,
}: AddCommentParams) {
if (shouldPlaySound) {
playSound(SOUNDS.DONE);
}
addActions({report, notifyReportID, ancestors, timezoneParam, currentUserAccountID, text, isInSidePanel, pregeneratedResponseParams});
addActions({report, notifyReportID, ancestors, timezoneParam, currentUserAccountID, text, isInSidePanel, pregeneratedResponseParams, reportActionID});
}

function reportActionsExist(reportID: string): boolean {
Expand Down
10 changes: 9 additions & 1 deletion src/libs/telemetry/activeSpans.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,16 @@ function cancelAllSpans() {
}
}

function cancelSpansByPrefix(prefix: string) {
for (const [spanId] of activeSpans.entries()) {
Copy link
Contributor

@roryabraham roryabraham Feb 13, 2026

Choose a reason for hiding this comment

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

Per code style conventions:

Suggested change
for (const [spanId] of activeSpans.entries()) {
for (const [spanID] of activeSpans.entries()) {

if (spanId.startsWith(prefix)) {
cancelSpan(spanId);
}
}
}

function getSpan(spanId: string) {
return activeSpans.get(spanId);
}

export {startSpan, endSpan, getSpan, cancelSpan, cancelAllSpans};
export {startSpan, endSpan, getSpan, cancelSpan, cancelAllSpans, cancelSpansByPrefix};
5 changes: 4 additions & 1 deletion src/pages/inbox/ReportScreen.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ import {
isTaskReport,
isValidReportIDFromPath,
} from '@libs/ReportUtils';
import {cancelSpan} from '@libs/telemetry/activeSpans';
import {cancelSpan, cancelSpansByPrefix} from '@libs/telemetry/activeSpans';
import {isNumeric} from '@libs/ValidationUtils';
import type {ReportsSplitNavigatorParamList, RightModalNavigatorParamList} from '@navigation/types';
import {setShouldShowComposeInput} from '@userActions/Composer';
Expand Down Expand Up @@ -652,6 +652,9 @@ function ReportScreen({route, navigation, isInSidePanel = false}: ReportScreenPr

// We need to cancel telemetry span when user leaves the screen before full report data is loaded
cancelSpan(`${CONST.TELEMETRY.SPAN_OPEN_REPORT}_${reportID}`);

// Cancel any pending send-message spans to prevent orphaned spans when navigating away
cancelSpansByPrefix(CONST.TELEMETRY.SPAN_SEND_MESSAGE);
};
}, [reportID]);

Expand Down
33 changes: 21 additions & 12 deletions src/pages/inbox/report/ReportActionCompose/ReportActionCompose.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import lodashDebounce from 'lodash/debounce';
import noop from 'lodash/noop';
import React, {memo, useCallback, useEffect, useMemo, useRef, useState} from 'react';
import React, {memo, useCallback, useContext, useEffect, useMemo, useRef, useState} from 'react';
import type {BlurEvent, MeasureInWindowOnSuccessCallback, TextInputSelectionChangeEvent} from 'react-native';
import {View} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
Expand Down Expand Up @@ -38,6 +38,7 @@
import DomUtils from '@libs/DomUtils';
import FS from '@libs/Fullstory';
import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID';
import {rand64} from '@libs/NumberUtils';
import Performance from '@libs/Performance';
import {getLinkedTransactionID, getReportAction, isMoneyRequestAction} from '@libs/ReportActionsUtils';
import {
Expand All @@ -64,6 +65,7 @@
import AgentZeroProcessingRequestIndicator from '@pages/inbox/report/AgentZeroProcessingRequestIndicator';
import ParticipantLocalTime from '@pages/inbox/report/ParticipantLocalTime';
import ReportTypingIndicator from '@pages/inbox/report/ReportTypingIndicator';
import {ActionListContext} from '@pages/inbox/ReportScreenContext';
import {hideEmojiPicker, isActive as isActiveEmojiPickerAction, isEmojiPickerVisible} from '@userActions/EmojiPickerAction';
import {addAttachmentWithComment, setIsComposerFullSize} from '@userActions/Report';
import {isBlockedFromConcierge as isBlockedFromConciergeUserAction} from '@userActions/User';
Expand Down Expand Up @@ -91,7 +93,7 @@

type ReportActionComposeProps = Pick<ComposerWithSuggestionsProps, 'reportID' | 'isComposerFullSize' | 'lastReportAction'> & {
/** A method to call when the form is submitted */
onSubmit: (newComment: string) => void;
onSubmit: (newComment: string, reportActionID?: string) => void;

/** The report currently being looked at */
report: OnyxEntry<OnyxTypes.Report>;
Expand Down Expand Up @@ -165,6 +167,8 @@
canBeMissing: true,
});
const ancestors = useAncestors(transactionThreadReport ?? report);
const {scrollPosition} = useContext(ActionListContext);

/**
* Updates the Highlight state of the composer
*/
Expand Down Expand Up @@ -355,16 +359,20 @@
});
attachmentFileRef.current = null;
} else {
Performance.markStart(CONST.TIMING.SEND_MESSAGE, {message: newCommentTrimmed});
startSpan(CONST.TELEMETRY.SPAN_SEND_MESSAGE, {
name: 'send-message',
op: CONST.TELEMETRY.SPAN_SEND_MESSAGE,
attributes: {
[CONST.TELEMETRY.ATTRIBUTE_REPORT_ID]: reportID,
[CONST.TELEMETRY.ATTRIBUTE_MESSAGE_LENGTH]: newCommentTrimmed.length,
},
});
onSubmit(newCommentTrimmed);
const reportActionID = rand64();
const isScrolledToBottom = !scrollPosition?.offset || scrollPosition.offset < CONST.REPORT.ACTIONS.ACTION_VISIBLE_THRESHOLD;
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Feb 12, 2026

Choose a reason for hiding this comment

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

P2 Badge Use a real scroll offset before gating send-span start

The new isScrolledToBottom check depends on scrollPosition.offset, but this value is not wired to chat scroll events in this flow (repo-wide search shows setScrollPosition is only written in useReportScrollManager/index.native.ts, where it is reset to 0 during scrollToBottom). That makes the condition effectively always true (undefined/0), so spans are still started while the user is scrolled up and the off-screen orphan-span case this change targets still occurs.

Useful? React with 👍 / 👎.

Copy link
Contributor Author

@cristipaval cristipaval Feb 13, 2026

Choose a reason for hiding this comment

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

looking into it

Copy link
Contributor Author

@cristipaval cristipaval Feb 13, 2026

Choose a reason for hiding this comment

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

ok, changed the approach for getting the current scroll position

if (isScrolledToBottom) {
Performance.markStart(CONST.TIMING.SEND_MESSAGE, {message: newCommentTrimmed});
startSpan(`${CONST.TELEMETRY.SPAN_SEND_MESSAGE}_${reportActionID}`, {
Comment on lines +367 to +368
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Feb 12, 2026

Choose a reason for hiding this comment

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

P2 Badge Skip starting send spans for task-shortcut submissions

This starts a send-message span before calling onSubmit, but onSubmitComment can return early when handleCreateTask(text) succeeds in ReportFooter, so no comment with that reportActionID is created and TextCommentFragment never calls endSpan for it. In that task-creation path the span is only canceled later during screen cleanup, which produces canceled/orphan send-message telemetry samples.

Useful? React with 👍 / 👎.

Copy link
Contributor Author

@cristipaval cristipaval Feb 12, 2026

Choose a reason for hiding this comment

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

I think that's fine, we only want to measure the text messages

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Feb 13, 2026

Choose a reason for hiding this comment

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

P2 Badge Delay send span start until comment is actually posted

startSpan() is called before onSubmit(), but onSubmit can short-circuit without posting a comment (for example, ReportFooter.onSubmitComment() returns early when handleCreateTask(text) is true in src/pages/inbox/report/ReportFooter.tsx). In that flow no report action with this generated reportActionID is created, so TextCommentFragment never calls endSpan(...) for it; the span stays active until a later screen teardown and inflates send-message telemetry during long sessions.

Useful? React with 👍 / 👎.

Copy link
Contributor Author

@cristipaval cristipaval Feb 13, 2026

Choose a reason for hiding this comment

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

we cleanup when the user moves away from the report page, so we won't have inflated spans

name: 'send-message',
op: CONST.TELEMETRY.SPAN_SEND_MESSAGE,
attributes: {
[CONST.TELEMETRY.ATTRIBUTE_REPORT_ID]: reportID,
[CONST.TELEMETRY.ATTRIBUTE_MESSAGE_LENGTH]: newCommentTrimmed.length,
},
});
}
onSubmit(newCommentTrimmed, reportActionID);
}
},
[
Expand All @@ -378,6 +386,7 @@
personalDetail.timezone,
isInSidePanel,
onSubmit,
scrollPosition,

Check failure on line 389 in src/pages/inbox/report/ReportActionCompose/ReportActionCompose.tsx

View workflow job for this annotation

GitHub Actions / ESLint check 

Dependency "scrollPosition" is too broad. Use specific properties instead: scrollPosition?.offset, scrollPosition.offset

Check failure on line 389 in src/pages/inbox/report/ReportActionCompose/ReportActionCompose.tsx

View workflow job for this annotation

GitHub Actions / ESLint check 

Dependency "scrollPosition" is too broad. Use specific properties instead: scrollPosition?.offset, scrollPosition.offset

Check failure on line 389 in src/pages/inbox/report/ReportActionCompose/ReportActionCompose.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Dependency "scrollPosition" is too broad. Use specific properties instead: scrollPosition?.offset, scrollPosition.offset

Check failure on line 389 in src/pages/inbox/report/ReportActionCompose/ReportActionCompose.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Dependency "scrollPosition" is too broad. Use specific properties instead: scrollPosition?.offset, scrollPosition.offset
],
);

Expand Down
3 changes: 2 additions & 1 deletion src/pages/inbox/report/ReportFooter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ function ReportFooter({
const targetReportAncestors = useAncestors(targetReport);

const onSubmitComment = useCallback(
(text: string) => {
(text: string, reportActionID?: string) => {
const isTaskCreated = handleCreateTask(text);
if (isTaskCreated) {
return;
Expand All @@ -203,6 +203,7 @@ function ReportFooter({
currentUserAccountID: personalDetail.accountID,
shouldPlaySound: true,
isInSidePanel,
reportActionID,
});
},
// eslint-disable-next-line react-hooks/exhaustive-deps
Expand Down
6 changes: 4 additions & 2 deletions src/pages/inbox/report/comment/TextCommentFragment.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,10 @@ function TextCommentFragment({fragment, styleAsDeleted, reportActionID, styleAsM

useEffect(() => {
Performance.markEnd(CONST.TIMING.SEND_MESSAGE, {message: text});
endSpan(CONST.TELEMETRY.SPAN_SEND_MESSAGE);
}, [text]);
if (reportActionID) {
endSpan(`${CONST.TELEMETRY.SPAN_SEND_MESSAGE}_${reportActionID}`);
}
}, [text, reportActionID]);

// If the only difference between fragment.text and fragment.html is <br /> tags and emoji tag
// on native, we render it as text, not as html
Expand Down
Loading