Skip to content

Conversation

@jpinkney-aws
Copy link
Contributor

@jpinkney-aws jpinkney-aws commented Oct 1, 2024

Problem:

  • It's not clear what's meant by cwsprChatTimeToFirstChunk, cwsprChatTimeBetweenChunks, cwsprChatFullResponseLatency
  • We don't have any client latency metrics

Solution:

  • Clarify the descriptions of cwsprChatTimeToFirstChunk, cwsprChatTimeBetweenChunks, cwsprChatFullResponseLatency
  • Implement cwsprTimeToFirstDisplay, cwsprChatTimeBetweenDisplay, cwsprChatFullDisplayLatency metrics

Other changes:

  • Instead of instrumenting everything via a traceId instead use the tabId as the unique identifier and store a traceId when we get it inside of the uiEventRecorder. That way we can associate events from a tab to the trace without needing to pass the trace around the entire application.
  • Lock stopChatMessageTelemetry on the tabId just in case something calls that twice for the same event

Risks:

  • All flows from start of chat message -> end of chat message need to be instrumented otherwise an addChatMessage event wont be sent.
  • RIght now:
    • Right click context menu works
    • Normal chat message work
    • Follow up button works
    • Are there any other flows that we need to know about?

License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

@jpinkney-aws jpinkney-aws requested a review from a team as a code owner October 1, 2024 18:27
@jpinkney-aws jpinkney-aws requested a review from leigaol October 1, 2024 18:27
@github-actions
Copy link

github-actions bot commented Oct 1, 2024

This pull request modifies code in src/ but no tests were added/updated. Confirm whether tests should be added or ensure the PR description explains why tests are not required.

@github-actions
Copy link

github-actions bot commented Oct 1, 2024

This pull request implements a feature or fix, so it must include a changelog entry. See CONTRIBUTING.md#changelog for instructions.

})
}
/**
* We've received an answer for a tabID and this message has
Copy link
Contributor

@leigaol leigaol Oct 2, 2024

Choose a reason for hiding this comment

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

Can you double check this callback? Is this code path executed when the first valid response is rendered to the customer chat panel or is it something else?

Specifically, does chat panel start rendering response when it receives item.type === ChatItemType.ANSWER_PART or item.type === ChatItemType.ANSWER

Copy link
Contributor

Choose a reason for hiding this comment

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

Once this is merged, can you update relevant documentation? Thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The chat panel starts rending the response when it sees the answer part (lines 203-215). When we receive ChatItemType.ANSWER it means the message response is fully finished

@jpinkney-aws jpinkney-aws marked this pull request as draft October 3, 2024 15:57
@jpinkney-aws
Copy link
Contributor Author

Moved this back to draft so its not merged. They now want separate metrics for user latency and server latency

Problem:
- It's not clear what's meant by cwsprChatTimeToFirstChunk, cwsprChatTimeBetweenChunks, cwsprChatFullResponseLatency
- We don't have any client latency metrics

Solution:
- Clarify the descriptions
- Implement cwsprTimeToFirstDisplay, cwsprChatTimeBetweenDisplay, cwsprChatFullDisplayLatency metrics
@jpinkney-aws jpinkney-aws force-pushed the jpinkney-aws/fix-chunk branch from 10bfebe to 10e3ec8 Compare October 7, 2024 19:52
@jpinkney-aws jpinkney-aws changed the title fix(telemetry): Use message sent time in CWSPRTimeToFirstChunk and CWSPRChatFullResponseLatency telemetry(amazonq): Add client latency metrics for amazon q chat Oct 7, 2024
@jpinkney-aws jpinkney-aws marked this pull request as ready for review October 7, 2024 20:09
@jpinkney-aws jpinkney-aws requested a review from a team as a code owner October 7, 2024 20:09
@jpinkney-aws jpinkney-aws merged commit de9d7cf into master Oct 8, 2024
11 of 16 checks passed
@jpinkney-aws jpinkney-aws deleted the jpinkney-aws/fix-chunk branch October 8, 2024 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants