[AS] [Telemetry] Telemetry session is not being set while we're on the AS panel

RESOLVED DUPLICATE of bug 1331059

Status

()

P1
normal
RESOLVED DUPLICATE of bug 1331059
2 years ago
2 years ago

People

(Reporter: Grisha, Assigned: Grisha)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MobileAS])

(Assignee)

Description

2 years ago
Looking at the data in redash, we can see that individual probes are firing as expected and rich AS data is in place, but somehow we're not setting the session name.

Here's the debug query: https://sql.telemetry.mozilla.org/queries/2083/source
I had problems with finding sessions in re:dash in an other bug too. Maybe there's a problem with sessions in general.
(Assignee)

Updated

2 years ago
Assignee: nobody → gkruglov
Status: NEW → ASSIGNED
(Assignee)

Comment 2

2 years ago
Well, on a surface it would appear that session are working, somewhat.

And perhaps the most interesting, here is a breakdown of which sessions actually make it along with the probes:
https://sql.telemetry.mozilla.org/queries/2094/source

~0.2%: ["reader"]
~0.017%: ["awesomescreen","reader"]
~8%: ["awesomescreen"]
~0.2%: ["awesomescreen","firstrun"]
~90%: []
~0%: ["awesomescreen","firstrun","reader"]
~0%: ["firstrun","reader"]
~1.8%: ["firstrun"]

So, most probes that fire don't have a session set at all, and some are clearly incorrect. E.g. some A-S probes fire with a session "awesomescreen".
(Assignee)

Comment 3

2 years ago
Comment 2 shows nightly data (and is missing a paragraph, it seems). Release and beta have similar breakdowns - no session is about 90-92%, "awesomescreen" by itself is about 7-8%. Note that sessions such as "settings", "frecency", "homepanel:*", "search_activity" don't make an appearance at all, even though we start/stop them in the code.
(Assignee)

Comment 4

2 years ago
Unless I'm missing something, looking through UITelemetry.jsm and friends (TelemetryController, TelemetrySend, etc), we pretty much send off ping data as is, with session data intact. So it seems like there might be something else at play here? Our sessions are named something like "awesomescreen.1", and I think that is what we send off in a telemetry ping. And yet ".1" gets stripped off the session name at some point.
(Assignee)

Comment 5

2 years ago
A workaround which should unblock building dashboards/queries for A-S data is to filter for it using something like: ... WHERE extras like "%fx_account_present%" ....

This relies on the fact that every A-S telemetry ping will have this extras field present, and no other telemetry is currently using anything like this.

Queries using this hack will be easily switchable to a proper sessions-based way of filtering once that's working.
(Assignee)

Comment 6

2 years ago
Georg, I wonder if you might help shed some light here. As far as I can tell, we're sending off telemetry pings with session data intact from the devices, but somewhere along the way "sessions" part of the payload is getting lost. It's not every payload that's affected - some of the sessions are unaffected and show up in redash (see Comment 2). One thing that seems to be in common (although I haven't verified it yet) is that the "lost" session names have a form of "activitystream.1:newtab", and the ones that make it look like "awesomebar.1", i.e. there's no colon-separated suffix. Could there be some server-side processing involved that's filtering certain sessions out?
Flags: needinfo?(gfritzsche)
Frank, this sounds like this could be an ETL issue with the mobile job for android_events_v1?
Flags: needinfo?(gfritzsche) → needinfo?(fbertsch)
Yes, this is a construct of the android_events ETL job. I'm not totally sure what the purpose was, but you can see[0] that the code is basically doing (note this is cleaned up a bit):

> for session in event["sessions"]:
>   if "experiment.1:" in session:
>     experiments.append(safe_str(session[13:]))
>   elif "firstrun.1" in session:
>     sessions.add("firstrun")
>   elif "awesomescreen.1" in session:
>     sessions.add("awesomescreen")
>   elif "reader.1" in session:
>     sessions.add("reader")

We can absolutely change it - I'm going to be doing some data engineering work on mobile stuff this Q, so we can add in stuff like this no problem.

[0] https://github.com/mozilla-services/data-pipeline/blob/master/reports/android-events/android-events.ipynb
Flags: needinfo?(fbertsch)
Oh, this explains why I couldn't find any 'search_activity' sessions too. I guess it won't be possible (or too 'expensive') to recover the sessions for past events?
(In reply to Sebastian Kaspari (:sebastian) from comment #10)
> Oh, this explains why I couldn't find any 'search_activity' sessions too. I
> guess it won't be possible (or too 'expensive') to recover the sessions for
> past events?

For one-off analysis, we could do something with a subset of historical data and it would still be statistically accurate. Backfill of these changes for all data would not be something I'd recommend.
(Assignee)

Comment 12

2 years ago
(In reply to Frank Bertsch [:frank] from comment #8)
> Yes, this is a construct of the android_events ETL job. I'm not totally sure
> what the purpose was, but you can see[0] that the code is basically doing
> (note this is cleaned up a bit):
> 
> > for session in event["sessions"]:
> >   if "experiment.1:" in session:
> >     experiments.append(safe_str(session[13:]))
> >   elif "firstrun.1" in session:
> >     sessions.add("firstrun")
> >   elif "awesomescreen.1" in session:
> >     sessions.add("awesomescreen")
> >   elif "reader.1" in session:
> >     sessions.add("reader")
> 
> We can absolutely change it - I'm going to be doing some data engineering
> work on mobile stuff this Q, so we can add in stuff like this no problem.
> 
> [0]
> https://github.com/mozilla-services/data-pipeline/blob/master/reports/
> android-events/android-events.ipynb

The client code which generates this telemetry makes an assumption that a session name will make it intact as it is constructed. We have a version number, allowing us to slice sets of events accordingly (granted, all of the sessions are currently at v1). We also have a concept of a session suffix. E.g. an event might have a session name such as "homepanel.1:1234-1234-1234", specifying an ID of the specific home panel which is active for the event, helping us to further narrow down scope of events.

This processing code doesn't seem to be in agreement. It would be good to understand why it's there, and perhaps the reason is irrelevant now, and we can let our session names remain intact going forward? This will let us add new session names without a need to make changes to the pipeline. We'll need to be careful and adjust any queries which currently depend on this processing to happen.
(In reply to :Grisha Kruglov from comment #12)
> (In reply to Frank Bertsch [:frank] from comment #8)
> The client code which generates this telemetry makes an assumption that a
> session name will make it intact as it is constructed. We have a version
> number, allowing us to slice sets of events accordingly (granted, all of the
> sessions are currently at v1). We also have a concept of a session suffix.
> E.g. an event might have a session name such as
> "homepanel.1:1234-1234-1234", specifying an ID of the specific home panel
> which is active for the event, helping us to further narrow down scope of
> events.
> 
> This processing code doesn't seem to be in agreement. It would be good to
> understand why it's there, and perhaps the reason is irrelevant now, and we
> can let our session names remain intact going forward? This will let us add
> new session names without a need to make changes to the pipeline. We'll need
> to be careful and adjust any queries which currently depend on this
> processing to happen.

Grisha, I made Bug 1331059 to track these changes.
(Assignee)

Updated

2 years ago
Depends on: 1331059
(Assignee)

Updated

2 years ago
See Also: → bug 1331091
(Assignee)

Comment 14

2 years ago
I'm going to close this bug in lieu of the data pipeline bug, since that's where the changes are going to land.
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1331059
You need to log in before you can comment on or make changes to this bug.