Add SNS logpoints #152
Add SNS logpoints #152
Conversation
ear-dev
changed the title
| const { postChatUrl, room: { _id: rid } = {} } = store.state; | ||
|
|
||
| const loggerPayload = { | ||
| room_id: rid, | ||
| category: 'Survey', | ||
| action: 'link_clicked', | ||
| properties: {}, | ||
| event_type: 'customer_action', | ||
| }; | ||
| Livechat.sendLogsToSns(loggerPayload); | ||
|
|
There was a problem hiding this comment.
How about we maintain all kinds of log payloads in some enum or something in one file and use it everywhere? And also may be creating a generator function that only takes rid and enum and returns the expected payload. So that it looks nice and clean and short 😁
There was a problem hiding this comment.
I have one more PR to add tab IDs, will add this there.
src/lib/idleTimeout.js
Outdated
| const loggerPayload = { | ||
| room_id: rid, | ||
| category: 'Chat Session', | ||
| action: 'closed', | ||
| properties: { | ||
| close_method: 'timeout', | ||
| }, | ||
| event_type: 'session', | ||
| timestamp: new Date(), | ||
| }; | ||
| Livechat.sendLogsToSns(loggerPayload); |
There was a problem hiding this comment.
@ear-dev Here close_method is timeout, but it does not say using app or widget. Should we add one more property for it?
There was a problem hiding this comment.
@Shailesh351 do you mean webApp vs. mobileApp? Or do you mean the timeout was set by the widget vs. set by the Salesforce app?
There was a problem hiding this comment.
And.... are we distinguishing between escalated timeout vs. visitor abandonment timeout?
There was a problem hiding this comment.
@ear-dev I mean the session got timed out from widget timer or timer running in the RC App
There was a problem hiding this comment.
And.... are we distinguishing between escalated timeout vs. visitor abandonment timeout?
The log point for escalated timeout will be in the App
There was a problem hiding this comment.
Thanks @Shailesh351
We only use the timer in the app, so I think this is fine because I do not suspect that we will ever want to use the timer in the widget.
My only question is how we will distinguish between VA timeout and SF timeout? I understand that the events come from different sources, but on the back end is there a way to distinguish? Or do we need to add something? i.e. I don't think we have any indication in our payload to distinguish if the event is from the widget, the RC server, or one of the RC.apps?
I'm thinking that the tracking plan is missing a distinction between VA and App timeout, and that's what we should fix...... thoughts?
There was a problem hiding this comment.
We can directly add a log point for the timeout in the SF App for SF timeout. And for the VA timeout may be server will have handle on it or in the SF app we receive close chat event and we can use it somehow for identifying the VA timeout
There was a problem hiding this comment.
@bhardwajaditya @ear-dev Logs are being pushed to SNS Topic successfully 🎉
@bhardwajaditya In the Chat Session closed event, it logs when the confirmation popup is shown but not when the user confirms to close the chat. Can you please check that?
|
@Shailesh351 fixed the bug you pointed out. |
There was a problem hiding this comment.
Looks good to me!
@bhardwajaditya when @ear-dev merges SDK PR (mostly on this Thursday) please update the dependency to multiple-clients branch
As mentioned in the above comments @bhardwajaditya is going to move all the log payloads to common place along with tab_id PR
|
@bhardwajaditya @Shailesh351 Up to now we have not been merging SNS Logger PRs, while we get the feature together. However, it might be time to start doing so....... but is this PR fully backwards compatible? If so, then it should be safe to merge while we get all the other stuff ready to go. Thanks. |
|
Yes @ear-dev This is just an additional feature, we are not modifying any functionalities. So we are good to go. |
|
@Shailesh351 @ear-dev I have added tab behaviour and ID changes in a PR against feature branch. Can't add you as an reviewer there. Please review |
|
@Shailesh351 please review so we can merge the baditya feature branch and then merge this PR. |
|
@bhardwajaditya please update the SDK dependency in the PR to use our regular default branch. |
[IMPROVE] Add tab behaviour logs and Tab_id for logs
ear-dev
changed the title
Closes: WideChat/Rocket.Chat#1207