-
Notifications
You must be signed in to change notification settings - Fork 16
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
feat: Add additional metrics for message tracking [CONSVC-1660] #330
Conversation
* `notification.message.expired` -- In flight message has no TTL, endpoint is unavailable. * `notification.message.stored` -- message stored for later transmission * `notification.message.retrieved` -- message extracted from storage * `ua.notification.sent` -- sent a notification to the websocket endpoint Issue: CONSVC-1660
metrics | ||
.incr_with_tags("ua.notification.sent") | ||
.with_tag("source", source) | ||
.send(); |
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.
A thought: we could kill the ua.notification.topic
above for a topic tag here like the other new metrics are doing
autopush/src/client.rs
Outdated
@@ -600,36 +603,37 @@ where | |||
} | |||
let now = ms_since_epoch(); | |||
let elapsed = (now - webpush.connected_at) / 1_000; | |||
let (ua_result, metrics_os, metrics_browser) = parse_user_agent(&user_agent); | |||
let ua_info = UserAgentInfo::from(user_agent.as_ref()); |
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.
Sorry one last thing, you shouldn't need to parse it again:
let ua_info = UserAgentInfo::from(user_agent.as_ref()); | |
let ua_info = webpush.ua_info.clone(); |
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.
Thanks! I realized that we don't need to pass the user_agent string anymore, so that's also been removed.
notification.message.expired
-- In flight message has no TTL, endpoint is unavailable.notification.message.stored
-- message stored for later transmissionnotification.message.retrieved
-- message extracted from storageua.notification.sent
-- sent a notification to the websocket endpointIssue: CONSVC-1660