1601016 - TelemetrySessionId annotation is documented but was never intentionally populated

Status: RESOLVED FIXED

(Toolkit :: Telemetry, task)

Description

["Saptarshi Guha[:joy]"]

As per this query: https://sql.telemetry.mozilla.org/queries/66640/source the field crash.payload.metadata.telemetry_session_id is hardly ever present. It is sometimes useful if you want to ignore all crashes coming from a given session id.

Query is

with a as (
select date(submission_timestamp) as date,
count(*) as n1, 
sum(case when crash.payload.metadata.telemetry_session_id is null then 1 else 0  end) as n2
from telemetry.crash
where DATE(submission_timestamp)>='2019-11-30'
group by 1
order by 1
)
select date, n1, n2, n2/n1*100 as PctOfPingsWithNullSessionID
from a

Output is

2019-11-30	1,691,274	1,690,897	99.98	
2019-12-01	1,643,166	1,643,027	99.99	
2019-12-02	2,558,759	2,558,367	99.98	

Comment 1

[William Lachance (:wlach)]

Gabriele, do you know why this might be? Looking at the crash ping code, it does appear like we are setting the value...

https://searchfox.org/mozilla-central/rev/8bc24752246aeac8a9aed566cf1caccf88d97d11/toolkit/crashreporter/client/ping.cpp#141

Comment 2

[Gabriele Svelto [:gsvelto]]

I've done a quick search on crash-stats and I noticed something odd related to this: content process crashes seem to have the field set, main process crashes don't. I've opened only a few dozen crashes for inspection so I can't be 100% sure but this might point to an issue with the crash reporting logic.

Comment 3

[Gabriele Svelto [:gsvelto]]

Oh yeah, I remembered what's going on: the session ID field is part of the ping payload, not the metadata field, so we explicitly remove it from the metadata. So the question is, why do we have pings with that field present under the metadata field?

Comment 4

[Gabriele Svelto [:gsvelto]]

I've double-checked that we remove the field even for content crash pings so this is quite odd. Could you check which versions of Firefox are sending the crash pings with the field set?

Comment 5

[Tim Smith (inactive) 👨‍🔬 [:tdsmith]]

(In reply to Gabriele Svelto [:gsvelto] from comment #3)

Oh yeah, I remembered what's going on: the session ID field is part of the ping payload, not the metadata field.

Got it: per bug 1187270 and https://searchfox.org/mozilla-central/rev/62a130ba0ac80f75175e4b65536290b52391f116/toolkit/components/crashes/CrashManager.jsm#681,697, payload.sessionId refers to the crash session.

We should probably clarify the crash ping docs.

Here's a query showing Firefox versions sending crash pings with metadata.telemetrySessionId in the last week -- there's a lot of 47 users in there (yikes) but also a bunch of recent builds: https://sql.telemetry.mozilla.org/queries/66993/source

process_type is always null, which is a little suspicious.

Comment 6

[Tim Smith (inactive) 👨‍🔬 [:tdsmith]]

(In reply to Tim Smith 👨‍🔬 [:tdsmith] from comment #5)

Here's a query showing Firefox versions sending crash pings with metadata.telemetrySessionId in the last week -- there's a lot of 47 users in there (yikes) but also a bunch of recent builds: https://sql.telemetry.mozilla.org/queries/66993/source

I looked at a couple of these pings and there's a mismatch between application.version and environment.build.version. I guess we actually care which version of Firefox is assembling the pings (application.version) and not which version of Firefox actually crashed (environment.build.version).

Looking at these by application.version, everything is quite old indeed: https://sql.telemetry.mozilla.org/queries/66996/source

So I think current versions of Firefox correctly do not report a telemetry_session_id.

Comment 7

[Tim Smith (inactive) 👨‍🔬 [:tdsmith]]

Oh, hilarious -- in fact, we only receive TelemetrySessionId annotations in the case where a crash from a "new" browser (containing bug 1187270) is being picked up from an "old" browser (before bug 1187270) that doesn't know it's supposed to delete the annotation. This was never intentionally populated.

Comment 8

[Tim Smith (inactive) 👨‍🔬 [:tdsmith]]

Attachment: Bug 1601016 - Clarify crash ping docs r?gsvelto

Make clear that payload.sessionId belongs to the crashing session, not the assembling session.

The TelemetrySessionId crash annotation was never intentionally populated.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4a6e133bc8de
Clarify crash ping docs r=gsvelto

Comment 10

[Cosmin Sabou [:CosminS]]

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