682869 - add Telemetry::AccumulateTimeDelta and use it

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Description

[Nathan Froyd [:froydnj]]

There are a number of places where telemetry probes are used to accumulate millisecond intervals.  We should have a separate function to make it obvious what's going on.

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: patch to add AccumulateTimeDelta and update callsites

Here's a patch to add the function and update appropriate callers.  I was unsure of whether to do the two things as separate patches, or one big ball of mud.  I'm sure somebody will straighten me out.

I'm also not sure who to pester to look at tweaks like this.

Comment 2

[Nathan Froyd [:froydnj]]

Comment on attachment 556583 [details] [diff] [review]
patch to add AccumulateTimeDelta and update callsites

Taras, can you review the telemetry parts and/or point me at who else should be reviewing the rest?

Comment 3

[(dormant account)]

Comment on attachment 556583 [details] [diff] [review]
patch to add AccumulateTimeDelta and update callsites

Next time please submit api change + code updates in 2 separate patches. As we discussed on irc, instead of passing TimeStamp::Now() use default parameter value or method overloading that implies Now().

Comment 4

[Nathan Froyd [:froydnj]]

Attachment: introduce AccumulateTimeDelta

Comment 5

[Nathan Froyd [:froydnj]]

Attachment: update places to use AccumulateTimeDelta

Comment 6

[(dormant account)]

Comment on attachment 565216 [details] [diff] [review]
introduce AccumulateTimeDelta

>+void AccumulateTimeDelta(ID id, TimeStamp start, TimeStamp end=TimeStamp::Now());

Add spaces around =, r+ with that

Comment 7

[Nathan Froyd [:froydnj]]

Comment on attachment 565217 [details] [diff] [review]
update places to use AccumulateTimeDelta

Flagging people for review on the use-it-everywhere patch.

Comment 8

[Jason Duell]

Comment on attachment 565217 [details] [diff] [review]
update places to use AccumulateTimeDelta

Review of attachment 565217 [details] [diff] [review]:
-----------------------------------------------------------------

Good stuff.  Thanks Nathan.

Comment 9

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 565217 [details] [diff] [review]
update places to use AccumulateTimeDelta

Review of attachment 565217 [details] [diff] [review]:
-----------------------------------------------------------------

Looks reasonable enough to me.

Comment 10

[Nathan Froyd [:froydnj]]

Attachment: introduce Telemetry::AccumulateTimeDelta

Updated with taras's requested spaces and real commit info.  Carrying over r+.

Comment 11

[Nathan Froyd [:froydnj]]

Attachment: update places to use AccumulateTimeDelta

Updated patch with r= info and such.  Carrying over r+.

Comment 12

[Nathan Froyd [:froydnj]]

Think everything's GTG for checkin-needed.  Flagging.

Comment 13

[Dão Gottwald [:dao]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/d6b9a54956c0
http://hg.mozilla.org/integration/mozilla-inbound/rev/e4edd20737fb

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d6b9a54956c0
https://hg.mozilla.org/mozilla-central/rev/e4edd20737fb