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

ref(telemetry): Add progress tag and updateProgress function #461

Merged
merged 5 commits into from
Sep 18, 2023

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Sep 15, 2023

This PR adds progress tracking to telemetry collection to identify, where in the respective wizard users cancel or drop off. This is done by updating the value of the progress tag in the following cases:

  • At the very beginning, start is set
  • At the very end of a successful run, finished is set
  • Whenever traceStep(step, callback) is called, the value of step is set.

Additionally, this PR adds an exported updateProgress function that can be used manually to update the progress if it makes sense.
(This is the result of a slack thread RE progress tracking)

#skip-changelog

src/telemetry.ts Outdated

let stepCounter = -1;
export function updateProgress(step: string) {
setTag('progress', `${++stepCounter}-${step}`);
Copy link
Member

Choose a reason for hiding this comment

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

I am wondering if our stats will get funky if we introduce new steps 🤔

Copy link
Member Author

@Lms24 Lms24 Sep 15, 2023

Choose a reason for hiding this comment

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

Hmm yes, that's a gooid point. I'm also a bit unsure about the counter tbh. In both cases, things will be messed up when steps change/are added/removed. We probably need to group by release to get this somewhat reliable but then again it's hard to compare improvements over releases...

Copy link
Member

Choose a reason for hiding this comment

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

Do we need the numeric value in the tag?

Copy link
Member

Choose a reason for hiding this comment

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

I could also imagine this being problematic in case there's different paths through the wizard.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I removed the counter.

Copy link
Member

@lforst lforst left a comment

Choose a reason for hiding this comment

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

have raised my concern about whether this data will be funky if we iterate on the wizard but if you feel like this is useful information keep track of I will not block this.

@Lms24
Copy link
Member Author

Lms24 commented Sep 18, 2023

I'm also not too sure about this but it's just a tag. If this doesn't work, we can still remove it/stop using it in our dashboard.

@Lms24 Lms24 merged commit cc01664 into master Sep 18, 2023
9 checks passed
@Lms24 Lms24 deleted the lms/ref-telemetry-progress branch September 18, 2023 14:18
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.

3 participants