Closed Bug 1550457 Opened 5 years ago Closed 5 years ago

GleanDebugActivity is broken in Fenix

Categories

(Data Platform and Tools :: Glean: SDK, defect, P1)

Product:
Data Platform and Tools
Component:
Glean: SDK
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: travis_, Assigned: Dexter)

References

Details

(Whiteboard: [telemetry:mobilesdk:m?])

Attachments

(2 files)

Fix GleanDebugActivity not always running
62 bytes, text/x-github-pull-request
Details | Review
Always terminate GleanDebugActivity when early returning PR
62 bytes, text/x-github-pull-request
Details | Review

After doing extensive testing on release Fenix builds, I think there is something in the way that Fenix has implemented Glean that is preventing the GleanDebugActivity from working correctly. I think this has something to do with how Fenix wraps Glean and treats it somewhat like a service.

Here's what I found... First, the only way I could get GleanDebugActivity to tag a ping and actually see that ping in the Debug View site, was to call the CLI command when Fenix was completely stopped (i.e. not running in the background at all). Otherwise, it would act like it was going to send the ping, but it would stop on the log output line of "Collecting baseline" similar to this:

05-08 13:56:08.568 7455-7486/org.mozilla.fenix D/glean/PingMaker: Collecting baseline
05-08 13:56:08.872 1983-3834/system_process I/WindowManager: Screenshot max retries 4 of Token{ef75be7 ActivityRecord{15af5a6 u0 org.mozilla.fenix/mozilla.components.service.glean.debug.GleanDebugActivity t77}} appWin=Window{21137e u0 Starting org.mozilla.fenix} drawState=3
05-08 13:56:09.448 1983-2242/system_process I/ActivityManager: START u0 {flg=0x10000000 cmp=org.mozilla.fenix/mozilla.components.service.glean.debug.GleanDebugActivity (has extras)} from uid 0 on display 0
05-08 13:56:09.513 7455-7455/org.mozilla.fenix I/glean/GleanDebugActivit: Setting debug config Configuration(serverEndpoint=https://incoming.telemetry.mozilla.org, userAgent=Glean/0.52.0 (Android), connectionTimeout=10000, readTimeout=30000, maxEvents=500, logPings=true, httpClient=mozilla.components.lib.fetch.httpurlconnection.HttpURLConnectionClient@98d8a79, pingTag=test-travis-fenix1, channel=null)

But, after those log lines, nothing else would happen towards collecting, storing, and scheduling/uploading the ping.

I don't know for sure, but it seems like Fenix wasn't handling the Intent from the GleanDebugActivity quite the same if it was already running.

Whiteboard: [telemetry:mobilesdk:m?]

Something else that I have noticed is I cannot seem to force send the metrics ping. Maybe it's being blocked by the scheduling restrictions of that ping? Same thing for activation ping, I cannot get it to send outside of the one time when the application first runs.

I'm not really sure if these are outside of the scope of what we were expecting the GleanDebugActivity to do.

I wasn't able to reproduce this problem with the baseline ping. However, I found two different problems:

  1. under some circumstances, the GleanDebugActivity would not get started again if the app was already open
  2. if re-triggered, the activity would cause onPause to be called and then a baseline ping/events ping to be generated even if they were not triggered manually.
Assignee: nobody → alessio.placitelli
See Also: → 1551700

There's another aspect to this: for some reason, sometimes, whenever I run the GleanDebugActivity using adb I get baseline and events pings generated, because the app detects that we're going to background.

(In reply to Travis Long from comment #1)

Something else that I have noticed is I cannot seem to force send the metrics ping. Maybe it's being blocked by the scheduling restrictions of that ping? Same thing for activation ping, I cannot get it to send outside of the one time when the application first runs.

I'm not really sure if these are outside of the scope of what we were expecting the GleanDebugActivity to do.

For baseline and events, I usually do the following:
[1] open app w/ Glean
[2] background app (which sends ping)

If I do the same with "--es sendPing metrics", pings do get sent, but only a baseline and an events ping (not a metrics ping).
However, if do the following:
[1] open app manually
[2] run Glean command:

adb shell am start -n org.mozilla.fenix/mozilla.components.service.glean.debug.GleanDebugActivity --ez logPings true --es sendPing metrics  --es tagPings rpapa-test-081

Then, the metrics ping gets sent immediately (without backgrounding the app)

expecting metrics ping when backgrounding the app sends the following:
https://debug-ping-preview.firebaseapp.com/pings/ac6d97f3-6a75-41dd-993f-173be1608066/rpapa-test-080

expecting metrics ping when app is first opened manually (then Glean command run) sends the following:
https://debug-ping-preview.firebaseapp.com/pings/4809b2b1-015a-4f0a-be29-f18294fafdbb/rpapa-test-081

(In reply to Richard Pappalardo [:rpapa][:rpappalardo] from comment #7)

For baseline and events, I usually do the following:
[1] open app w/ Glean
[2] background app (which sends ping)

If I do the same with "--es sendPing metrics", pings do get sent, but only a baseline and an events ping (not a metrics ping).
However, if do the following:
[1] open app manually
[2] run Glean command:

adb shell am start -n org.mozilla.fenix/mozilla.components.service.glean.debug.GleanDebugActivity --ez logPings true --es sendPing metrics  --es tagPings rpapa-test-081

Then, the metrics ping gets sent immediately (without backgrounding the app)

This is very useful! I didn't notice this, and I think I now know what's going on. In Glean, we use the ProcessLifeCycleOwner to detect when the app is going to background/foreground. As per its documentation:

Other lifecycle events will be dispatched with following rules: ProcessLifecycleOwner will dispatch Lifecycle.Event.ON_START, Lifecycle.Event.ON_RESUME events, as a first activity moves through these events. Lifecycle.Event.ON_PAUSE, Lifecycle.Event.ON_STOP, events will be dispatched with a delay after a last activity passed through them

So, when starting the app through GleanDebugActivity, we might be messing with the way lifecycle its being tracked, so we sent the extra pings because we think we're going to "background".

Coming back at this after clearing out bug 1552798, I think that there was some miscomunication. Let me clarify the behaviour below (we slightly changed the docs to make this clearer!).

(In reply to Richard Pappalardo [:rpapa][:rpappalardo] from comment #7)

For baseline and events, I usually do the following:
[1] open app w/ Glean
[2] background app (which sends ping)

What do you mean, here, by open the "app w/ Glean"? Using the adb command with no sendPing? If so, this behaviour is correct and expected. As documented here and here, pings are either send because of a (1) pre-defined behaviour (product or Glean) or (2) because of calling the adb command with sendPing.

Moving the application to background falls in (1), it's a Glean-defined ping behaviour: both the baseline and the events pings are always sent when going to background.

If I do the same with "--es sendPing metrics", pings do get sent, but only a baseline and an events ping (not a metrics ping).
However, if do the following:
[1] open app manually
[2] run Glean command:

adb shell am start -n org.mozilla.fenix/mozilla.components.service.glean.debug.GleanDebugActivity --ez logPings true --es sendPing metrics  --es tagPings rpapa-test-081

Then, the metrics ping gets sent immediately (without backgrounding the app)

This falls in (2), described above. Whenever the adb command with sendPing is used a ping is immediately collected (and potentially sent).
In general, we don't need to move the product to background to get pings sent. We only need to move the product to background if we want to trigger pings without the adb command. And this would only work for the pings (i.e. baseline and events) that behave like that.

@Richard, does this clarify the things?

Flags: needinfo?(rpappalardo)

(In reply to Alessio Placitelli [:Dexter] from comment #11)

What do you mean, here, by open the "app w/ Glean"? Using the adb command with no sendPing? If so, this behaviour is correct and expected.

If I recall correctly, when I was previously running this command with sendPing, it wouldn't arrive in the dashboard until I backgrounded it. However,... Since the activation ping has been fixed, this seems to be working as expected now (don't know if that's related or not).

This falls in (2), described above. Whenever the adb command with sendPing is used a ping is immediately collected (and potentially sent).
In general, we don't need to move the product to background to get pings sent. We only need to move the product to background if we want to trigger pings without the adb command. And this would only work for the pings (i.e. baseline and events) that behave like that.
@Richard, does this clarify the things?

Yes, and thank you for the thorough explanations!

:bsurd also mentioned that he'd spoken with you in regards to a better testing work flow, which I'll try to summarize here:
[1] start app manually
[2] do whatever it is that you wanna do to log a specific ping for testing
[3] then run the (Glean) adb command (rather than starting the app w/ the adb command)

Running it to also start the app already sends the pings, thus the reason why you don't see some pings logged
Cause there is no data to be send as the app just started and maybe didn't even finish properly initializing
(I noticed in the docs also, that this can happen for example when baseline data is sent before duration has been calculated)

Flags: needinfo?(rpappalardo)

(In reply to Richard Pappalardo [:rpapa][:rpappalardo] from comment #12)

(In reply to Alessio Placitelli [:Dexter] from comment #11)

What do you mean, here, by open the "app w/ Glean"? Using the adb command with no sendPing? If so, this behaviour is correct and expected.

If I recall correctly, when I was previously running this command with sendPing, it wouldn't arrive in the dashboard until I backgrounded it. However,... Since the activation ping has been fixed, this seems to be working as expected now (don't know if that's related or not).

This seems unrelated, but it's hard to tell without a logcat. If you happen to trigger this again, would you kindly file a bug and attach a logcat trace with the related STR?

This falls in (2), described above. Whenever the adb command with sendPing is used a ping is immediately collected (and potentially sent).
In general, we don't need to move the product to background to get pings sent. We only need to move the product to background if we want to trigger pings without the adb command. And this would only work for the pings (i.e. baseline and events) that behave like that.
@Richard, does this clarify the things?

Yes, and thank you for the thorough explanations!

My pleasure!

:bsurd also mentioned that he'd spoken with you in regards to a better testing work flow, which I'll try to summarize here:
[1] start app manually
[2] do whatever it is that you wanna do to log a specific ping for testing
[3] then run the (Glean) adb command (rather than starting the app w/ the adb command)

Yes, this is one of the two flows we support. It's not really "better", just different!

Running it to also start the app already sends the pings, thus the reason why you don't see some pings logged
Cause there is no data to be send as the app just started and maybe didn't even finish properly initializing
(I noticed in the docs also, that this can happen for example when baseline data is sent before duration has been calculated)

Exactly. If you start the application with the adb command and also provide a sendPing argument, it will send the ping as soon as the application is started: data may or may not be available, depending on the specific ping behaviour. We can sort of determine what to expect from the baseline ping, but metrics, events and custom pings are something that heavily depend on the product using Glean.

I'm closing off this bug as all the related PRs were merged and we clarified the testing flow a bit more. Please don't hesitate to reach out or file a new bug if you stumble upon anything weird!

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: