Refactor EventTimer + Changes to commercial metrics

[Jakeii]

What does this change?

Refactor event-timer.ts with the following changes

• Remove google analytics reporting as we don't use it any longer
• Change the way that timers are stored (use a map rather than an array/object combo)
• Remove the first timer, and measure specific slots instead (see below)
• Introduce some measures/deltas to replace some of the timers/marks

Why?

First timer removal

Using the first for each event has led to some inconsistencies in the data, for roughly 21% of data points the timers are recorded in the wrong order, we believe could be caused by the first event of each type coming from different slots in some circumstances.

In most cases the first slot is simply top-above-nav anyway, and using pageviews where that isn't the case could be adding some undesirable noise/variables to the data.

We also have very little insight into what goes on on ad slots further down the page, we we will add reporting for inline1 (often a video ad) and inline2 (floated right on desktop).

Deltas & new measurements

Most of the timers we don't actually even use for analysis, only adOnPage generally. Changing most of them to deltas will give us more useful information more easily about specific pieces of code that we're interested in e.g. how long does prebid, defineSlot (which incorporates IAS), loadSlot (fetching the ad from GAM) take.

Hopefully deltas will be superior timestamps as they won't be subjected to variance from before the timer we're interested in has even started.

What we're doing in diagram form, we are keep commercialStart and adOnPage (green) as timestamps, throwing away the rest (red) and adding delta's (in blue)

Summary of deltas

commercialBoot			-> time for commercial modules to load
googletagInit			-> Time for googletag to load
${slot}-defineSlot		-> Defining the slot (`window.googletag.defineSlot`) and running IAS
${slot}-prepareSlot		-> mostly Prebid.js, but includes our own code around it
${slot}-prebid			-> the time for prebid auction to happen
${slot}-fetchAd			-> the time for the ad to be fetched from GAM
${slot}-adRender		-> the time for the ad to render, from when it's in view to the ad appearing on the page (should be loadSlot + adRender)

Summary of timers

commercialStart         -> Pretty much the earliest point in the commercial code
commercialModulesLoaded -> All async modules in commercial.standalone.ts loaded
${slot}-adSlotReady		-> Ad slot defined and ready to render
${slot}-adRenderStart 	-> Ad (scrolled) in view and loading
${slot}-adOnPage    	-> The ad appeared on the page

[changeset-bot]

🦋 Changeset detected

Latest commit: 2c28c61

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@guardian/commercial	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

🚀 0.0.0-beta-20230706105953 published to npm as a beta release

[github-actions]

🚀 0.0.0-beta-20230706154936 published to npm as a beta release

[chrislomaxjones]

Thanks for the very helpful diagram!

[chrislomaxjones]

Nice one!

[arelra]

Nice work and thanks for the thorough explanation! Just some comments on naming conventions that we can discuss.

[arelra]

We can talk about this but I was hoping to align the naming convention for performance marks across commercial, webex and cmp to make data analysis easier.

I was thinking of a prefix e.g.:

gu.commercial or gu.dcr or gu.cmp

And then adopting a naming pattern with a consistent separator that goes generic -> detailed from left to right e.g.

gu.commercial.boot.start
gu.commercial.boot.end
gu.commercial.boot.duration

gu.commercial.slot.define.top-above-nav.start
gu.commercial.slot.define.top-above-nav.end
gu.commercial.slot.define.top-above-nav.duration

or

gu.dcr.island.hydration.mostviewed.start
gu.dcr.island.hydration.mostviewed.end
gu.dcr.island.hydration.mostviewed.duration

These are just ideas but 'duration' aligns with the measures we are computing here.

If we can agree on a naming pattern that works across the teams it would be ideal if we can implement it in this PR for commercial (or another before releasing).

[arelra]

Have raised this as a separate cross-team RFC so this is not a blocker to this PR.

We can update the naming as a follow up PR.

[arelra]

Very minor but I think in the PR diagrams loadSlot and prepareSlot were used interchangeably?

[Jakeii]

I changed my mind about the name at some point, will update the diagram

[domlander]

Could you update the handbook when this is live?

https://docs.google.com/document/d/10Mxxta6ypxAduVXJFp19uRoWtALQmAubdKtBwq7AqZ0/edit#heading=h.w0c6dt3g4krt

[arelra]

Great work! 🎉

[chrislomaxjones]

Looking forward to seeing these timers roll out!