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

Bug 1918149 - Add measurements of the impact of disabing Glean event batching to the docs #2999

Merged
merged 2 commits into from
Nov 18, 2024

Conversation

travis79
Copy link
Member

No description provided.

@travis79 travis79 requested a review from a team as a code owner November 13, 2024 20:37
@travis79 travis79 requested review from badboy and removed request for a team November 13, 2024 20:37
Copy link
Member

@badboy badboy 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 some hesitation r+'ing this.

For one I'm not 100% sure this is the right place to put it.
This is something very specific, while the rest talks about the general approach.
At the very least this should be marked as such, maybe using an info block? That would make it clear it's looking at the current real product state.

Second, are we comfortable publishing these numbers as is? (They are now somewhat public in this PR).
How quickly will they be out of date? Is this relevant to us/others reading this page in 3 months/6 months/etc.
Maybe having that in the bug, not in the docs, is "good enough"?

Comment on lines 17 to 19
As of November 2024, Desktop Release:
Median user per day: 67 events / 3 pings
The impact of turning on one event per ping based on the median user would result in an increase of approximately 21 times more event ping volume.
Copy link
Member

Choose a reason for hiding this comment

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

We should find a better formatting for this. mdbook treats this all as a single line, so at the very least it needs a . (dot) after 3 pings or something (see rest in review comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried to clean it up a bit, see what you think now.

@travis79
Copy link
Member Author

In regards to where this should live: Yes, I agree it's not exactly comfortable here but it is the closest to the information that would make it relevant (turning off event batching), so this is where I saw it living for now.

In regards to the numbers? Yeah, any measurement I made would at some point be out of date but I think that the general trend of number of events to number of events pings being about 20-40 : 1 ratio is going to be fairly consistent, and anyone thinking of turning this on I would hope would take more recent measurements into consideration.

@travis79 travis79 merged commit c0e5bd1 into main Nov 18, 2024
29 of 30 checks passed
@travis79 travis79 deleted the Bug1918149 branch November 18, 2024 13:30
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