1345108 - Reduce resolution for timestamp in pingSender's crash pings

Status: VERIFIED FIXED

(Toolkit :: Telemetry, defect, P1)

Description

[Chris H-C :chutten]

While I was writing the patch for bug 1338627 I noticed that, when assembling the crash ping, pingSender uses a per-second-resolution timestamp for creationTimestamp.

No big deal, crash manager does the same thing, right?

Well, maybe.

crash manager's crashping assembly happens at some pseudo-random time in the future, making its creationTimestamp somewhat unrelated to the time that Firefox crashed.

pingSender on the other hand runs a far more deterministic time after a crash occurs, making its creationTimestamp somewhat _related_ to the time that Firefox crashed.

If, as mentioned in bug 1338627comment#3, we are deliberately trying to be vague about when crashes happened, we should teach pingSender to be vague about its crash pings' creationTimestamps. (by truncating or fuzzing)

Comment 1

[Alessio Placitelli [:Dexter]]

The timestamp comes from the crashmanager who builds up the ping, so we should fix it there.

Comment 2

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to Alessio Placitelli [:Dexter] from comment #1)
> The timestamp comes from the crashmanager who builds up the ping, so we
> should fix it there.

The CrashManager is not affected since it creates the ping at the earliest a minute or two after the crash. The issue is specifically with the crashreporter client that creates a ping only a few instants after the crash happened:

https://dxr.mozilla.org/mozilla-central/rev/e03e0c60462c775c7558a1dc9d5cf2076c3cd1f9/toolkit/crashreporter/client/ping.cpp#

This causes the ping "creationDate" to be practically the same as the real time of the crash which is a piece of information we don't want to be public.

Comment 3

[Alessio Placitelli [:Dexter]]

Attachment: Bug 1345108 - Reduce resolution for timestamp in crashreporter's crash pings.

Review commit: https://reviewboard.mozilla.org/r/126030/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/126030/

Comment 4

[(not currently active) Ted Mielczarek]

Comment on attachment 8854041 [details]
Bug 1345108 - Reduce resolution for timestamp in crashreporter's crash pings.

https://reviewboard.mozilla.org/r/126030/#review128654

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7e09e95fef84
Reduce resolution for timestamp in crashreporter's crash pings. r=ted

Comment 6

[Alessio Placitelli [:Dexter]]

Comment on attachment 8854041 [details]
Bug 1345108 - Reduce resolution for timestamp in crashreporter's crash pings.

Approval Request Comment
[Feature/Bug causing the regression]: 1310703
[User impact if declined]: Crash pings generated by the crashreporter will report the exact time a crash happened.
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: This will be verified on Nightly as soon as it lands.
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: This patch should be applied after bug 1345153 and bug 1348250.
[Is the change risky?]: No risk.
[Why is the change risky/not risky?]: This is truncating the creationDate string when generating crash pings from the crashreporter, using code that already lived in the crashreporter.
[String changes made/needed]: None

Comment 7

[Alessio Placitelli [:Dexter]]

Abe, could you please quickly validate this as soon as it hits Nightly?

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7e09e95fef84

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Seems that it doesn't apply to aurora
     

grafting 390651:7e09e95fef84 "Bug 1345108 - Reduce resolution for timestamp in crashreporter's crash pings. r=ted"
merging toolkit/components/telemetry/docs/data/crash-ping.rst
merging toolkit/crashreporter/client/ping.cpp
 warning: conflicts while merging toolkit/crashreporter/client/ping.cpp! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue

Comment 10

[Alessio Placitelli [:Dexter]]

Attachment: [AURORA-REBASED] Reduce the resolution of the creationDate

Rebased patch. Is it ok to uplift this?

Comment 11

[Abe - QA (:Abe_LV)]

QE Verified as fixed. 

Test cases and runs are available here: https://public.etherpad-mozilla.org/p/_Bug_1345108

Comment 12

[Gerry Chang [:gchang]]

Comment on attachment 8854041 [details]
Bug 1345108 - Reduce resolution for timestamp in crashreporter's crash pings.

Fix timestamp in crash pings and was verified. Aurora54+.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a213b5ee1fba