PERF-813 Capture diagnostic details

[ajhyndman]

Capture the last mutation and last image load data in the detail object reported by TTVC. These can be useful to report to assist diagnosing regressions in the field.

[hongrich]

Added one suggestion

[hongrich]

So lastImageLoadTimestamp would be -1 if there are no image loads... 🤔

lastTimestamp: number;
lastImageLoadTarget?: HTMLElement;
lastMutation?: MutationRecord;

What do think of something like this? lastTimestamp will always be available, and either lastImageLoadTarget or lastMutation would exist but not both. This might be a slightly more ergonomic API.

[ajhyndman]

The reason I split it this way is that otherwise there's no way to know whether the image or the mutation was the last event.

lastTimestamp would be equivalent to end on the main object as well.

[ajhyndman]

I guess I should also document the behaviour we decide on in this PR, wherever we land.

[hongrich]

MutationRecord and HTMLElement are different enough that the code handling the callback would need to check which one it is and handle them differently anyways. I think here are some of the options:

A:

lastImageLoadTimestamp: number;
lastImageLoadTarget?: HTMLElement;
lastMutation?: TimestampedMutationRecord;

B:

lastTimestamp: number;
lastImageLoadTarget?: HTMLElement;
lastMutation?: MutationRecord;

C:

lastTimestamp: number;
lastMutationOrTarget: MutationRecord | HTMLElement;

For A, it's possible that there are values in all 3 fields, so the callback handler need to compare the timestamp in both of sets of data to see which one is later.
For B, timestamp is always available and either the target or mutation would be available and not both, so the callback handler only need to do a falsey/truthy check.
For C, pretty similar to B, but callback handler would do instanceof check instead of a falsey/truthy and the union type makes it more obvious that only one of the two is available and not both.

B or C feel like a better API for the callback handler to handle and exposes fewer of the internal implementation.

[ajhyndman]

Oh, sorry. I think I understand your point better now.

Let me prompt you with a further question though:

I'm hearing an assumption in your proposal that any consumer will only want to know about the last mutation or the last image loaded. Is that really the case? Even for our own debugging purposes, it seems to me that it could be useful to know that no images loaded during the pageload, or to know how much of a delta there is between image content loading and the last mutation.

If you don't think that's useful, you're right. We can adopt options B or C (we can even drop lastTimestamp without losing information). But this is also a debug affordance. It seems okay to me to give a little more power to consumers at the cost of a little more verbose API.