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

Add telemetry for method modeling panel #3041

Merged
merged 5 commits into from
Nov 1, 2023

Conversation

charisk
Copy link
Contributor

@charisk charisk commented Nov 1, 2023

Adds some telemetry events for interactions with the modeling panel.

The method-modeling-set-multiple-modeled-methods is a bit generic, but we can improve that in the future if we want to.

Checklist

N/A:

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

@charisk charisk requested a review from a team as a code owner November 1, 2023 11:41
Copy link
Contributor

@robertbrignull robertbrignull left a comment

Choose a reason for hiding this comment

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

I have a few comments, but they're all very subjective so please do say if you feel otherwise.

There are also maybe some other interactions, but I don't know if we want to emit telemetry for them or not:

  • Changing index (i.e. view next/previous modeling)
  • Adding a new modeling
  • Removing a modeling

Though adding/removing would also trigger the catch-all "setMultipleModeledMethods" event.

@charisk
Copy link
Contributor Author

charisk commented Nov 1, 2023

I have a few comments, but they're all very subjective so please do say if you feel otherwise.

There are also maybe some other interactions, but I don't know if we want to emit telemetry for them or not:

  • Changing index (i.e. view next/previous modeling)
  • Adding a new modeling
  • Removing a modeling

Though adding/removing would also trigger the catch-all "setMultipleModeledMethods" event.

Thanks. I've stopped being lazy and added events for those events too.

Copy link
Contributor

@robertbrignull robertbrignull left a comment

Choose a reason for hiding this comment

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

One typo, otherwise LGTM

@charisk charisk enabled auto-merge (squash) November 1, 2023 14:54
@charisk charisk merged commit 3f90564 into main Nov 1, 2023
@charisk charisk deleted the charisk/method-modeling-telemetry branch November 1, 2023 15:15
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.

2 participants