feat: more analytics #161
feat: more analytics #161
Conversation
|
No linked issues found. Please add the corresponding issues in the pull request description. |
src/handlers/push_message.rs
Outdated
| #[cfg(feature = "analytics")] | ||
| if let Some(mut message_info) = analytics_option { | ||
| message_info.status = status; | ||
| message_info.success = status >= 200 && status < 300; |
There was a problem hiding this comment.
Nice to have, but isn't it enough with just status in the table?
There was a problem hiding this comment.
my intention with the success field was to give echo server the responsibility to determine when a request is successful or not, given it has more context on what's being done. Given it's just
message_info.success = status >= 200 && status < 300;
I don't mind dropping the column.
Consider if in the future things becomes more complicated like if status == 2xx AND some other condition then it would be a problem for data to determine, with this column you control whether the outcome was successful or not.
There was a problem hiding this comment.
I've removed it for now
src/handlers/push_message.rs
Outdated
| if let Some(mut message_info) = analytics_option { | ||
| message_info.status = status; | ||
| message_info.success = status >= 200 && status < 300; | ||
| message_info.response_message = Some(response.body().to_string()); |
There was a problem hiding this comment.
Is the presence of 'response_message' the only differentiator between a request and a response?
In that case, can we have a field that tells explicitly which is which to avoid potential downstream issues?
There was a problem hiding this comment.
response message was so we could provide context on errors
There was a problem hiding this comment.
ok, I think we still need to be able to differentiate between requests and responses
|
Builds should be all sorted now, waiting for CI & Re-review before merge + deploy |
Description
Includes requested additional analytics
Resolves # (issue)
How Has This Been Tested?
Build locally
Due Diligence