1074468 - Add callids to request.summary log for "GET /calls"

Status: RESOLVED FIXED

(Hello (Loop) :: Server, defect)

Description

[Katie Parlante]

A couple of our most important high level metrics are about counting # of unique callers (recurring, per day, total, etc.). We do this by counting unique ids on the endpoints "GET /calls" and "POST /calls" (initiating and receiving calls). We don't want to count calls if we know they've failed, however, so we want to filter out calls that are not associated with a successful websocket setup.

As of https://bugzilla.mozilla.org/show_bug.cgi?id=1046236 we're logging websocket successes/failures, along with the callId. callId shows up on logs of "POST /calls", so we can link those uids to success/failure of the callId. We don't have that information for "GET /calls", so we can't associate a uid with any call that is initiated by clicking on a url.

Presumably "GET /calls" will have a list of calls with callIds, it would be great if we could log this list as an array of ids.

Comment 1

[Rémy Hubscher (:natim)]

Well the problem with GET /calls is that you can have more than one call at the time.
Also why can't we add the callId on POST /calls/:token instead ?

This would be the exact same thing as POST /calls for direct calls but for callUrls.

Comment 2

[Katie Parlante]

(In reply to Rémy Hubscher (:natim) from comment #1)
> Well the problem with GET /calls is that you can have more than one call at
> the time.
Yeah, can we do a list of the callids? The point is to associate the uid logged with "GET /calls" with a successful callid in a later websocket.summary log line, so that we can count that uid as a unique user with a successful call.

> Also why can't we add the callId on POST /calls/:token instead ?
The callId is being logged here, but we're not logging any uid on this line.

Right now, we don't have a way to associate callIds with uids in the url generation scenario.

Here's my understanding:
1. Alice generates a url and emails it to Bob (we'll see "POST /call-url" in the logs, including Alice's uid)
2. Bob clicks on the url, and sees the web page for creating a call (we'll see "GET /calls/:token" in the logs, no uid for Bob or Alice)
3. Bob clicks the button to initiate the call (we'll see "POST /calls/:token" in the logs, and the callId, but no uid for Bob or Alice)
4. Alice's client gets notified of the call, rings (we'll see "GET /calls" in the logs, including Alice's uid, but no callId)
5. Alice accepts the call (the websocket connection gets set up successfully, we'll get a websocket.summary success log, with the callId)

We can look at the websocket.summary success line and count a successful call. We can't currently associate that success with a uid for either Alice or Bob. We've accepted that we'll miss Bob (no uid, no good way to count him uniquely), but we'd really like to count Alice.

The current logic that populates the dashboard *does* count Alice, whether or not step 5 happens properly. We'd like to only count her only if the call is set up successfully.

I suppose another approach is to log the token on the websocket.summary line, and use token as a way to link to Alice's uid, as I think we get both from logging step 1.

Comment 3

[Rémy Hubscher (:natim)]

Ok I completly forgot about GET /calls/:token and was thinking of GET /calls

Also we don't have any callId on the GET /calls/:token we have on POST /calls/:token.

What I would propose is to log both Bob and Alice uid as well as the callId on POST /calls/:token

Does this would fix your problem?

This doesn't prevent us for also logging the callToken on the websocket.summary

Comment 4

[Katie Parlante]

(In reply to Rémy Hubscher (:natim) from comment #3)
> Ok I completly forgot about GET /calls/:token and was thinking of GET /calls

Yeah, that indicates Bob has clicked on the url, so we use it to count url clicks, but we don't use it for any of the "successful calls" metrics (recurring, daily, total, etc.)

> Also we don't have any callId on the GET /calls/:token we have on POST
> /calls/:token.

I don't think we need the uid on GET /calls/:token

> What I would propose is to log both Bob and Alice uid as well as the callId
> on POST /calls/:token

Do we have a uid for Bob? I thought we didn't have a uid for the url clicker. If you have Alice's uid here and log it, that would work.

> Does this would fix your problem?

I think so. Do we have both (or all) party's uid for POST /calls/? (For the scenario where both sides of the call are known). If so, we should log both sides in this case as well.

> This doesn't prevent us for also logging the callToken on the
> websocket.summary

Sure, sounds like logging the callToken here is a good idea.

Comment 5

[Rémy Hubscher (:natim)]

> Sure, sounds like logging the callToken here is a good idea.

Oh I remembered why we didn't do it at first, because we didn't want to hit the database to log it.

Comment 6

[Rémy Hubscher (:natim)]

So if I understand correctly this bug:

 - We want to log Alice userHmacId
 - We want to log the callId

This can helps to link callId with callee and success or failure.

Is that correct?

Comment 7

[Rémy Hubscher (:natim)]

Attachment: Link to Github PR — #225.

Comment 8

[Rémy Hubscher (:natim)]

FWIW in this patch I am logging Bob's UID if we have it.

Comment 9

[Alexis Metaireau (:alexis)]

Comment on attachment 8502473 [details] [review]
Link to Github PR — #225.

This needs tests in order to be merged.

Comment 10

[Katie Parlante]

(In reply to Rémy Hubscher (:natim) from comment #6)
> So if I understand correctly this bug:
> 
>  - We want to log Alice userHmacId
>  - We want to log the callId
> 
> This can helps to link callId with callee and success or failure.
> 
> Is that correct?

Yes, that is correct.

Comment 11

[Katie Parlante]

Comment on attachment 8502473 [details] [review]
Link to Github PR — #225.

This looks right to me

Comment 12

[Alexis Metaireau (:alexis)]

Assigning to tarek, okay for you?

Comment 13

[Tarek Ziadé (:tarek)]

https://github.com/mozilla-services/loop-server/commit/2ab1c07a16374a823d3c3a1fd7fd5ba9451219a9