Skip to content
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.

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

Chat: Add tracing #3168

Merged
merged 10 commits into from
Feb 15, 2024
Merged

Chat: Add tracing #3168

merged 10 commits into from
Feb 15, 2024

Conversation

philipp-spiess
Copy link
Contributor

@philipp-spiess philipp-spiess commented Feb 14, 2024

@philipp-spiess philipp-spiess requested a review from a team February 14, 2024 15:29
@abeatrix
Copy link
Contributor

WOOOO

if (inputText.match(/^\/reset$/)) {
return this.clearAndRestartSession()
}
return tracer.startActiveSpan('chat.submit', async (span): Promise<void> => {
Copy link
Contributor

Choose a reason for hiding this comment

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

noob question: should we start the active span from when the question is submitted (from the command menu, chat view, or commands), or we only want to trace when we handle the request that was sent here?
If i want to add the tracing from when the command was executed, so we can trace how long it takes before it arrives to this function, do i just add another active span to that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideally as early as possible but when the trace is started it does not happen in the same JavaScript thread (it happens in the UI thread and not the extension host) so I put it here instead. Do you think there is a long delay until it arrives here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think there is a long delay until it arrives here?
At least for commands, we fetch the context before they are sent here so it'd be cool to see those. I will add it to commands based off your branch and try it out!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, yeah for the simple chat provider I think this function is called pretty much immediately after the webview message arrives. Context fetching happens later

@abeatrix abeatrix mentioned this pull request Feb 14, 2024
Copy link
Contributor

@abeatrix abeatrix left a comment

Choose a reason for hiding this comment

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

🚀 thanks for setting this up again!!

@philipp-spiess philipp-spiess merged commit e45a39e into main Feb 15, 2024
17 checks passed
@philipp-spiess philipp-spiess deleted the ps/chat-traces branch February 15, 2024 11:32
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.

None yet

2 participants