Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fetch the gatheredTimestamp #4753

Closed
paulirish opened this issue Mar 14, 2018 · 4 comments · Fixed by #4783
Closed

Fetch the gatheredTimestamp #4753

paulirish opened this issue Mar 14, 2018 · 4 comments · Fixed by #4783

Comments

@paulirish
Copy link
Member

Right now we add a .generatedTime timestamp to the LHR that marks when runner is packaging up the LHR.

IMO, a more useful timestamp is when we start gathering; basically when we loaded the page. Beyond semantics, this probably only matters in the situations where gathering&auditing are done separately. Recently, I've been repeatedly regenerating the sample_v2.json, doing -A runs from the exact same artifacts. I'd argue it doesn't really make sense for the timestamp in the LHR to change.

(Implementation-wise: Collecting in the gatherrunner right when we grab the userAgent makes sense to me).

A few questions:

  1. Do ya'll agree this is a more useful timestamp?
  2. If we start exposing this, should we stop exposing the current .generatedTime? (including for the LHR-Lite)
  3. If we do that, we should probably rename this property. We could do just .startTime. And then the timings in core(perf): add timing instrumentation to measure runtime perf #3745 would be treated just like navStart & resource timing.

thoughts?

@patrickhulce
Copy link
Collaborator

  1. Yes
  2. Yes! ☂️ 👣 Things to fix on next breaking change #4333 :)
  3. I'm not a huge fan of .startTime. It makes some sense to keep the run time measurements distinct from absolute datetime that the thing happened. This field was renamed audit_time internally, FWIW, but I'm not a huge fan of that either. In my experience these types of fields in APIs are generally named *At, createdAt, auditedAt, fetchedAt, gatheredAt, any of those feel good?

@brendankenny
Copy link
Member

This definitely makes way more sense than LHR generation time. I'm fine with fetchedAt or gatheredAt or whatever variation, and they seem more self descriptive that startTime (fetchedTime or gatheredTime might better fit our existing style but I really don't care :).

Also agree with run time measurements being relative rather than absolute...I can't imagine why you would need an absolute time, and at worst you could reconstruct from the start time and elapsed times

@paulirish
Copy link
Member Author

paulirish commented Mar 14, 2018

K. I'm also into fetchedAt the most and gatheredAt second. Groovy. Sounds like we're good here then.

@ev1stensberg wanna take this?

i think this is the sequence:

  1. remove generatedTime from runner.
  2. as i mention above, mostly duplicate the pattern of how the useragent is collected and carried through. (though we can just grab new Date() .. no driver interaction necessary)
  3. where generatedTime is attached to the object in runner, place this timestamp as fetchedAt

@evenstensberg
Copy link
Contributor

Yep, picking this up. Thanks for letting me 👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants