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.

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

Jump to bottom

end-to-end performance tracking for code completion requests #3165

Merged
merged 4 commits into from
Jul 31, 2023
Merged

end-to-end performance tracking for code completion requests #3165

merged 4 commits into from
Jul 31, 2023

Conversation

robstryker
Copy link
Contributor

No description provided.

@robstryker
Fixes part of redhat-developer#2685 - end-to-end performance tracking…
f3b048f 
… for code completion requests

Signed-off-by: Rob Stryker <[email protected]>
rgrunber
rgrunber reviewed Jun 22, 2023
View reviewed changes
Copy link
Member

@rgrunber rgrunber left a comment

Choose a reason for hiding this comment

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

Overall, looks good. Haven't tried it yet, but just some small things I observed.

src/standardLanguageClient.ts Outdated Show resolved Hide resolved
src/standardLanguageClient.ts Outdated Show resolved Hide resolved
src/standardLanguageClient.ts Outdated Show resolved Hide resolved
src/standardLanguageClient.ts Outdated Show resolved Hide resolved
@robstryker robstryker mentioned this pull request Jun 22, 2023
Open
@robstryker
Cleanup for issue 2685
e01affc 
Signed-off-by: Rob Stryker <[email protected]>
@rgrunber rgrunber requested a review from fbricon June 23, 2023 17:00
rgrunber
rgrunber reviewed Jun 23, 2023
View reviewed changes
Copy link
Member

@rgrunber rgrunber left a comment

Choose a reason for hiding this comment

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

Overall looks good.

We're tied into the tracing language client (which is used by the standard language client) so we won't get any telemetry in lightweight (syntax) mode or while a project is switching over between syntax/standard on loading, but I guess we should be ok with that for now. I also confirmed this wouldn't pick up cancellations.

Wouldn't mind hearing @fbricon's thoughts.

src/standardLanguageClient.ts Outdated Show resolved Hide resolved
@rgrunber
Copy link
Member

rgrunber commented Jun 26, 2023 •
edited
Loading

After speaking to Fred, we decided to merge this after the end of June July release. Then we can decide how best to configure it.

@rgrunber
Copy link
Member

Can you also modify the USAGE_DATA.md file to mention the new telemetry.

"The number of results (eg. 20), whether an error occured (eg. false), and duration (in milliseconds) when code assist is activated"

@rgrunber rgrunber added this to the End August 2023 milestone Jul 27, 2023
@robstryker
Copy link
Contributor Author

updated usage_data file

@robstryker
Copy link
Contributor Author

Updated unnecessary cast

rgrunber
rgrunber approved these changes Jul 31, 2023
View reviewed changes
Copy link
Member

@rgrunber rgrunber left a comment

Choose a reason for hiding this comment

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

LGTM. I'll be merging soon.

@rgrunber rgrunber merged commit 8fa2464 into redhat-developer:master Jul 31, 2023
@rgrunber rgrunber changed the title Fixes part of #2685 - end-to-end performance tracking for code completion requests end-to-end performance tracking for code completion requests Jul 31, 2023
@rgrunber rgrunber mentioned this pull request Sep 8, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rgrunber rgrunber rgrunber approved these changes

@fbricon fbricon Awaiting requested review from fbricon

Assignees

@robstryker robstryker

Labels
telemetry
Projects
IDE Cloudaptors
Archived in project
Milestone
Early September 2023
Development

Successfully merging this pull request may close these issues.
2 participants
@robstryker @rgrunber