Giter Club home page Giter Club logo

Comments (11)

igrigorik avatar igrigorik commented on May 9, 2024 1

We discussed this on last week's call and @toddreifsteck pointed out that Edge runs the "add the entry to performance entry buffer" as soon as it's created and updates relevant attributes on the same record as they become available. This behavior is counter to what the current processing section specifies (add the entry once all attributes are finalized), but also provides a reasonable solution to the issue we're discussing here...

Adding the entry as soon as it is created provides similar semantics to NT1, which makes the upgrade path very simple. Applications can query the timeline to retrieve the navigation record and log it / beacon it at any point in the pageload cycle. If the navigation is aborted, for whatever reason, analytics is still able to collect nav timing data.


Also, a few related thoughts and considerations:

(1) Should we apply similar logic to ResourceTiming, and run the "add step" as soon as the fetch is initiated, instead of waiting until its done? The upside is consistency between NT and RT; this would address a related request for having a signal that a resource fetch has been initiated.

(2) How does all of the above affect PerformanceObserver? Should we similarly move the "queue step" to right after the record is created?

  • For Nav Timing this doesn't work that well, since the record would be emitted early in the page load and most analytics scripts probably won't yet be registered. I think this suggests that we should hold the record until all the attributes are finalized.
  • For Resource Timing, similar constraints apply for any/all fetches initiated as part of the initial page load.. You'd have to register very early to get those notifications. Further, if you get the early record, now you have a handle to an incomplete entry which you need... poll?

I think my proposal here would be:

  • Move the "add the X entry to performance buffer" step, for both Nav and Resource Timing, to immediately after the entry is created.
  • Keep the "queue the X entry to observer" step, for both Nav and Resource Timing at the end; observers will only see these records once the page load / resource load has finished.

WDYT?

from navigation-timing.

igrigorik avatar igrigorik commented on May 9, 2024

I'd be curious to know how existing integrations deal with this today, because even with NT1 ( which updates the same entry as the timestamps become available) you'd have the same issue:

  • Either you continue polling and sending partial records
  • Or, you wait until some abort signal and harvest all/as much data as you can

@bluesmoon @nicjansma @bmaurer @philipwalton how do you handle this today? My guess is it's the latter: wait until onunload, harvest the data, and beacon it then? Also, do you have a sense, from your telemetry, how common this case is?

from navigation-timing.

bluesmoon avatar bluesmoon commented on May 9, 2024

boomerang waits for onload before beaconing, but we also beacon if beforeunload fires before onload, and in all cases, we also collect the time when the DOMContentLoaded event fires. We have certainly seen cases where DOMContentLoaded fires but onload doesn't, but I can't say that it is very common.

from navigation-timing.

bmaurer avatar bmaurer commented on May 9, 2024

At facebook we use a "capped average" for our metrics. This means that we take:

AVG(time > 60 seconds ? 60 seconds : time)

the idea here is that we penalize ourselves for extremely slow requests but we don't allow them to skew the average too much since these requests are usually broken in ways that are difficult to fix.

Because of this, we report metrics back to our servers at the 60 second mark even if the load event hasn't occurred. Thus, it is important for us to have access to this timing information before the load event has occurred. We'll probably end up using the NT1 API until this can be addressed.

I can also attest that from our experience we've found that situations like the one @bryanmcquade mentions have serious impact on metrics reporting. Anything that makes it hard for slow requests to report back (eg large reporting payloads, data not being available until onload) skew metrics quite a bit.

from navigation-timing.

igrigorik avatar igrigorik commented on May 9, 2024

Ben, thanks for the feedback and context.

Because of this, we report metrics back to our servers at the 60 second mark even if the load event hasn't occurred. Thus, it is important for us to have access to this timing information before the load event has occurred. We'll probably end up using the NT1 API until this can be addressed.

How do you handle the abort case today? Based on above, if the page did not hit onload or 60s timeout, it would be omitted in your telemetry? Or, is there some other code path for the early abort case?

from navigation-timing.

bmaurer avatar bmaurer commented on May 9, 2024

We started using sendBeacon very early in the page to say "this trace is alive". So if we switched to NT2 we'd start seeing an increase in traces that are lost.

from navigation-timing.

igrigorik avatar igrigorik commented on May 9, 2024

As in, you periodically sendBeacon the NT1 object as the page is loading and then reconcile that on your backend? ... Not clear on how sendBeacon + early ping solve this today. 😕

from navigation-timing.

spanicker avatar spanicker commented on May 9, 2024

+1
This seems like a reasonable way to overcome this limitation.

from navigation-timing.

spanicker avatar spanicker commented on May 9, 2024

sakamoto / irori@ has raised the following concerns on the implementation review:

  1. the values of some fields in NT will now change depend on when they are queried, and there is no clear way to indicate that the value is "not available yet" vs. "not applicable" (e.g. redirectStart when there's no redirect) vs. zero

Proposal: can we use "-1" (or some pre-determined non-conflicting value) to indicate "not yet available" (as this is the new case we are adding with this change)?

  1. Some values aren't populated in a timely fashion due to different reasons (than the one we've introduced with this change) e.g. domainLookupStart will not be populated until final response is available, although domain lookup must be finished long before that.
    Is that okay?

For discussion details see: https://codereview.chromium.org/2647643004/

@irori @igrigorik @bryanmcquade

from navigation-timing.

igrigorik avatar igrigorik commented on May 9, 2024

the values of some fields in NT will now change depend on when they are queried, and there is no clear way to indicate that the value is "not available yet" vs. "not applicable" (e.g. redirectStart when there's no redirect) vs. zero

This is not a 'new' problem. NT1 consumers already have to deal with this, and AFAIK this has not been a problem in practice. For redirectStart in particular, note that we do set it to fetchStart in our processing algorithm (i.e., even if there is no redirect the value is non zero); we do reset it to 0 across redirects that don't pass TAO-algorithm, but we also increment redirectCount in the process, so once again, you can distinguish between "not available yet" vs "not applicable". Similar logic applies to other fields.

Proposal: can we use "-1" (or some pre-determined non-conflicting value) to indicate "not yet available" (as this is the new case we are adding with this change)?

That would make it incompatible with definition of DOMHighResTimestamp.

Some values aren't populated in a timely fashion due to different reasons (than the one we've introduced with this change) e.g. domainLookupStart will not be populated until final response is available, although domain lookup must be finished long before that.
Is that okay?

We should provide same behavior as we did with NT1.

from navigation-timing.

irori avatar irori commented on May 9, 2024

Okay, sounds like it's not going to be a problem in practice. Thanks!

from navigation-timing.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.