1349324 - Add main process start time to pings that use event telemetry

Status: RESOLVED FIXED

(Toolkit :: Telemetry, enhancement, P1)

Description

[bugzilla]

Some use cases of event telemetry benefit from knowing the (approximate) absolute time of the event. Since event timestamps are relative to the main process start time, we should add it to the main ping (and other ping types that want to use events)

Comment 1

[Mark Hammond [:markh] [:mhammond]]

(In reply to Sunah Suh [:sunahsuh] from comment #0)
> (and other ping types that want to use events)

Yeah, the sync ping will want this.

Comment 2

[Georg Fritzsche [:gfritzsche]]

This is a good point, there is probably interest in that outside of events as well.
We should make it clear though that this will be subject to clock issues (or try to fix it up through meta field use).

We already have sessionStartDate & subsessionStartDate in [1] as ISO dates, they are just currently truncated to daily precision.
How about we just make them more precise?
Is a precision of full minutes sufficient?
Or would we need seconds?

1: https://gecko.readthedocs.io/en/latest/toolkit/components/telemetry/telemetry/data/main-ping.html

Comment 3

[Georg Fritzsche [:gfritzsche]]

(In reply to Georg Fritzsche [:gfritzsche] [away March 24 - April 2] from comment #2)
> We already have sessionStartDate & subsessionStartDate in [1] as ISO dates,
> they are just currently truncated to daily precision.
> How about we just make them more precise?
> Is a precision of full minutes sufficient?
> Or would we need seconds?

Mark, can you answer to the required precision?

Comment 4

[Mark Hammond [:markh] [:mhammond]]

I think minutes would be fine for our purposes.

Comment 5

[Benjamin Smedberg]

FWIW, we are explicitly trying to keep low granularity of absolute dates, because granular timestamps make pings/users much more identifiable.

So my goal here as data steward is to pick something that is as coarse as possible: can we do this with hours?

Comment 6

[Mark Hammond [:markh] [:mhammond]]

(In reply to Benjamin Smedberg [:bsmedberg] from comment #5)
> So my goal here as data steward is to pick something that is as coarse as
> possible: can we do this with hours?

My goal is to be able to order events from different pings, and multiple pings with the same process start time makes this tricky. Is there a middle ground - is something like max(current_hour, minute_last_session_ended) or similar possible?

Comment 7

[Georg Fritzsche [:gfritzsche]]

(In reply to Mark Hammond [:markh] from comment #6)
> (In reply to Benjamin Smedberg [:bsmedberg] from comment #5)
> > So my goal here as data steward is to pick something that is as coarse as
> > possible: can we do this with hours?
> 
> My goal is to be able to order events from different pings, and multiple
> pings with the same process start time makes this tricky. Is there a middle
> ground - is something like max(current_hour, minute_last_session_ended) or
> similar possible?

Wasn't looking at round-trip time for sync over devices for diagnostics also desired?
That's where i thought you might need minutes or higher precision.

Comment 8

[Mark Hammond [:markh] [:mhammond]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #7)
> Wasn't looking at round-trip time for sync over devices for diagnostics also
> desired?
> That's where i thought you might need minutes or higher precision.

Yeah, I'd certainly *like* higher precision, but I think 1-hour granularity is fine for what we actually need today. There's no reason we couldn't revisit this later, right?

Comment 9

[Georg Fritzsche [:gfritzsche]]

(In reply to Mark Hammond [:markh] from comment #8)
> Yeah, I'd certainly *like* higher precision, but I think 1-hour granularity
> is fine for what we actually need today. There's no reason we couldn't
> revisit this later, right?

Right, lets go with hours for now then.

Comment 10

[Georg Fritzsche [:gfritzsche]]

Mark, i'll use this bug to do this for the main ping.
I filed bug 1353307 for adding this to the sync ping.

Comment 11

[Georg Fritzsche [:gfritzsche]]

Attachment: Use hourly precision for sessionStartDate & subsessionStartDate

Comment 12

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8854383 [details] [diff] [review]
Use hourly precision for sessionStartDate & subsessionStartDate

Requesting data-review for this change, see the changes to the .rst file.

Comment 13

[Alessio Placitelli [:Dexter]]

Comment on attachment 8854383 [details] [diff] [review]
Use hourly precision for sessionStartDate & subsessionStartDate

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

This looks good with observations below addressed.

::: toolkit/components/telemetry/TelemetryUtils.jsm
@@ +59,5 @@
>                      0, 0, 0, 0);
>    },
>  
>    /**
> +   * Takes a date and returns it trunctated to a date with hourly precision.

nit: "truncated"

::: toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
@@ +1228,5 @@
>    keyed.add("b", 1);
>  
>    // Trigger and collect environment-change ping.
>    gMonotonicNow = fakeMonotonicNow(gMonotonicNow + 10 * MILLISECONDS_PER_MINUTE);
> +  let startDay = TelemetryUtils.truncateToDays(now);

Is |startDay| still being used?

@@ +1246,5 @@
>    Assert.equal(ping.payload.histograms[COUNT_ID].sum, 1);
>    Assert.equal(ping.payload.keyedHistograms[KEYED_ID]["a"].sum, 1);
>  
>    // Trigger and collect another ping. The histograms should be reset.
> +  startDay = TelemetryUtils.truncateToHours(now);

Should this be |startHour = ...| ?

Comment 14

[Georg Fritzsche [:gfritzsche]]

Attachment: Use hourly precision for sessionStartDate & subsessionStartDate

Comment 15

[Georg Fritzsche [:gfritzsche]]

Attachment: Use hourly precision for sessionStartDate & subsessionStartDate

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e4a0ac84712
Use hourly precision for sessionStartDate & subsessionStartDate. r=dexter

Comment 17

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8854509 [details] [diff] [review]
Use hourly precision for sessionStartDate & subsessionStartDate

I missed reflagging on the new patch.
Requesting data review for this precision change, see the changes to the .rst file.

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4e4a0ac84712

Comment 19

[Benjamin Smedberg]

Comment on attachment 8854509 [details] [diff] [review]
Use hourly precision for sessionStartDate & subsessionStartDate

data-r+