feat: demote retry message, dump vapid claims.
#510
Conversation
This patch demotes the `retryable` `error!` message since it's not actionable. It also introduces a dump of the `VAPID` claims, extracting the `sub` from the header for easy parsing.
| let claims = vapid.token.split('.').collect::<Vec<&str>>(); | ||
| if let Some(claims_str) = claims.get(1) { | ||
| use base64::Engine; | ||
| info!( |
There was a problem hiding this comment.
What's the goal of this one? This won't emit to the logs currently if that was the intention because autoendpoint only logs at the warn level.
I know we've talked about emitting more such data in the future but I'm not sure exactly what and where to emit it yet -- as is this seems like it would too chatty (if it were a warn!).
There was a problem hiding this comment.
Huh. Doing a BigQuery search on ...autopush_prod_log.stdout shows that we're capturing log messages with INFO severity, (jsonPayload.severity = 6.0) for the Session info. I presume I'm looking at the wrong ones?
There was a problem hiding this comment.
As for the goal of this, I view it as the same sort of data we collect for the session info. We'd dump it to stdout and query it using Bigquery.
There was a problem hiding this comment.
autoconnect is set to info so you may only be seeing those (or maybe our severity is not trustworthy?)
There was a problem hiding this comment.
Perhaps, but I got those values from the Google docs
That said, the values we're seeing in the GCP logs do not appear to be the same ones we're generating here (The ones in the logs appear to be floats, these are integers), so maybe the docs are wrong and some experimentation is in order.
There was a problem hiding this comment.
But per the metric emitted a little later in this method we've had about 2k vapid notifications per second emitted around the time that I wrote this which isn't even peak.
We can merge this as is since it won't output to logs yet, but it probably makes more sense as a debug! for now (especially since that will remove the log call entirely at compile time w/ release_max_level_info).
There was a problem hiding this comment.
Fair. I'll demote to debug as well for this, just to keep things safer.
There was a problem hiding this comment.
Meh, if we're not using this, throw it behind a feature flag.
This patch demotes the
retryableerror!message since it's not actionable.It also introduces a dump of the
VAPIDclaims, extracting thesubfrom the header for easy parsing.Closes: SYNC-3996