Add mobile metrics #27045
Add mobile metrics #27045
Conversation
larkox
added
2: Dev Review
mm-cloud-bot
added
release-note
server/enterprise/metrics/metrics.go
Outdated
| Namespace: MetricsNamespace, | ||
| Subsystem: MetricsSubsystemClientsMobileApp, | ||
| Name: "mobile_load", | ||
| Help: "Duration of the time taken from when a user opens the app and the app finally loads all relevant information (milliseconds)", |
There was a problem hiding this comment.
I'm not sure if this is a typo copied from above (global_threads_load should be in seconds like the other web app metrics), but these should also be measured in seconds because that's the standard for Prometheus metrics. I ran into the same thing with the web app metrics too
There was a problem hiding this comment.
Yeah, I wrote this down when I first implemented it, and when we decided to go with seconds, I forgot to update it.
| Subsystem: MetricsSubsystemClientsMobileApp, | ||
| Name: "mobile_load", | ||
| Help: "Duration of the time taken from when a user opens the app and the app finally loads all relevant information (milliseconds)", | ||
| Buckets: []float64{1, 1.5, 2, 3, 4, 4.5, 5, 5.5, 6, 7.5, 10, 20, 25, 30}, |
There was a problem hiding this comment.
What range do you expect for these values to be in? Because of how Prometheus handles histograms, any values above the maximum will be treated as if it was the maximum, so we should make sure that it's rare for this to be longer than 30ms. That value seems much lower than I'd expect even when the app is loading quickly
There was a problem hiding this comment.
Also, what's your reasoning behind these specific sets of buckets? At first glance, i t feels a bit arbitrary that these 3 use different bucket sizes
There was a problem hiding this comment.
These are seconds and not milliseconds 😅 So 30s is way over the acceptable time, so anything above that we don't care the specifics. It is too much.
My tests done locally (with development build, may be way off) showed me that load was around the 5s mark. That is why I added more precision around that value (0.5s increases). Similar idea with the other two, where I was taking values of around 500ms and 200ms. Since the expected results are different, the buckets also have to be different.
server/enterprise/metrics/metrics.go
Outdated
| Help: "Duration of the time taken from when a user opens the app and the app finally loads all relevant information (milliseconds)", | ||
| Buckets: []float64{1, 1.5, 2, 3, 4, 4.5, 5, 5.5, 6, 7.5, 10, 20, 25, 30}, | ||
| }, | ||
| []string{"platform", "agent"}, |
There was a problem hiding this comment.
Do we need the agent label here? We only need that for the web app metrics to differentiate different browsers, but we don't really need it for mobile.
We could consider tracking the version of the mobile app instead, but that could also be added later
There was a problem hiding this comment.
The agent is not really needed, but the way it is implemented right now, the metric verification requires an agent. It is something we can look a bit deeper into.
server/enterprise/metrics/metrics.go
Outdated
| Namespace: MetricsNamespace, | ||
| Subsystem: MetricsSubsystemClientsMobileApp, | ||
| Name: "mobile_channel_switch", | ||
| Help: "Duration of the time taken from when a user opens the app and the app finally loads all relevant information (milliseconds)", |
There was a problem hiding this comment.
It seems like this description was accidentally copied from above
There was a problem hiding this comment.
I thought I updated these 🤦
server/enterprise/metrics/metrics.go
Outdated
| Namespace: MetricsNamespace, | ||
| Subsystem: MetricsSubsystemClientsMobileApp, | ||
| Name: "mobile_team_switch", | ||
| Help: "Duration of the time taken from when a user opens the app and the app finally loads all relevant information (milliseconds)", |
Summary
Add server code for mobile metrics
Ticket Link
NONE
Release Note