Embedded GA metrics

[porter-stripe]

Summary

• Distinguish between embedded in load events
• Add analytics for update API calls
• Track confirm calls

Motivation

• Embedded GA

Testing

• Updating existing tests
• New unit tests

Changelog

N/A

[yuki-stripe]

Why log here and not at the start of the update method?

[porter-stripe]

Good question! I think we should log when the update work actually begins not when it is scheduled. If we log it when it is scheduled (at the top of the function) it makes it harder to reason about the duration metric, since some will account for awaiting the previous update canceling and some won't. If we want consistency of the duration metric across all update events we should only measure the time the actual work associated with the update took, not accounting for any time it spent waiting for a previous update to cancel.

It's also convenient from an implementation standpoint with how we currently handle dates in the analytics helper. If we allow multiple update start events to be fired before the finished event it will overwrite the updateStartDate before the previous finished event could send.

If we fire the start event at the top we could get this
mc_embedded_update_started -> set's updateStartDate to x
mc_embedded_update_started -> set's updateStartDate to y
mc_embedded_update_finished -> reads updateStartDate set to y
mc_embedded_update_finished -> reads updateStartDate set to y

We could get around this by mapping update events and their start dates to a UUID so we know what start date to compute the duration on for all the finished events. But, putting the analytics in the Task ensures the ordering is correct/consistent which I prefer.

[yuki-stripe]

If we moved this to the start of update, duration represents the time it takes for the update method to finish. That feels quite simple and straightforward.

The total time the app spends waiting for update to finish also seems like a more useful duration metric. Like, why ignore the time spent waiting for previous update calls to finish? That's still time the app spends waiting and something we control (e.g. we could try to cancel in-flight tasks more aggressively).

Moving to the top has the additional benefit of capturing all possible errors.

Re: implementation - what if we keep track of the start time within the update method?

[yuki-stripe]

Hm can we not log the result immediately before this method returns?

[porter-stripe]

Correct me if I'm wrong, but I believe there is a race condition. By the time you reach logEmbeddedUpdateFinished(result:) at the end of the function, a second update could have already started its own update and logged a start event. There’s no guarantee about which event comes first unless you serialize the calls within the same Task. Logging right before the method returns can record an “update finished” event out of order, or with an incorrect start date, if another update began in between.

latestUpdateTask?.cancel()
_ = await latestUpdateTask?.value
// Define task

let updateResult = await currentUpdateTask.value
analyticsHelper.logEmbeddedUpdateFinished(result: updateResult)
// After we await the update task we cannot guarantee this code runs before the next update task starts and logs an update started event, resetting the updateStartDate.

As I write this out, maybe we should just map update calls to a UUID and a start date to avoid this, but logging all the update events within the Task seems the safest with the current way we handle dates in the analytics helper. Even with that approach, we could get two update start events sent before an update finishes event, putting all the analytics in the Task ensures the order is correct/deterministic and makes it easier to reason about the ordering of the events, so I slightly prefer putting them within the Task.

[yuki-stripe]

Imagine the merchant does the following

1. Call update
2. Call update again before (1) finishes.
3. Receive Update (1) result
4. Receive Update (2) result

I think you're saying the correct analytics ordering is:

1. update_started
2. update_finished
3. update_started
4. update_finished

That matches the ordering of the Tasks inside the update method.

I'm proposing (along with my other comment above) to model this analytic around the entire update method, not just the inner Task. This would make the ordering match the merchant's POV:

1. update_started
2. update_started
3. update_finished
4. update_finished

The ordering is still deterministic; update_started fires when the update API is called and update_finished fires when it returns.

[porter-stripe]

Sure we can do that 👍