Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 client metrics #751
Add client metrics #751
Changes from 8 commits
36460a1
ca3db46
d1ca053
efce9ac
b462c21
7e067aa
6656416
e8be503
a7ff258
1c9dbce
6b15c15
b1a23cd
eba5843
6da2986
1692691
67dfe4d
d205832
94df8c1
5e0bf36
7b334a4
0ee03f2
738672d
7638e57
2aa80fc
382152f
53b1c87
ca56401
f91dc82
a42059f
016e7ac
2562236
02a451d
abda30f
63fd563
06bc18a
7d1e7a8
b4d4170
f4e3947
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think this is not semantically correct. I'm not sure if it's a problem with the naming (how are we using it exactly in the webapp?) or with the code here. For reference: I would understand this if the metric would be called something like
model.ClientTimeToLogin
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, answering to your comment in the other thread (sorry, just read it now):
I'm ok adding a ticket to refine this to the backlog, but I'd still like to understand how the webapp uses this specific metric, I think I lack some context here, sorry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An explanation can be found here: https://web.dev/articles/ttfb and turns out my interpretation is also incorrect. Just sent a canonical way of measuring it with your suggestion 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's just use other/other for this. If we use all the platforms, then this becomes hard for a user to open a browser session and observe the "actual" client metrics in a live load test.
More context here: https://community.mattermost.com/core/pl/9y8cgyxxq7yq5yjorpqykbjkrw
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for bringing this up. I think this approach will strike good balance between team wanting to use load testing tool for the web client metrics and tool being used to measure impact of web client metrics with load test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whaaat? Are you saying there's an equal chance of Safari sending metrics than Chrome? :p
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vanilla macos users are quite a lot among devs to be fair :p