1517231 - Glean: Upload even though metrics are disabled?

Status: RESOLVED FIXED

(Toolkit :: Telemetry, enhancement, P1)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

I am adding glean to the reference browser here:
https://github.com/mozilla-mobile/reference-browser/pull/349

Even though I have metrics disabled via setMetricsEnabled() I still see uploads happening in the log:

> D  Ping upload: 200
> D  Ping successfully sent (200)

I was expecting no upload to happen in this case?

Comment 1

[Travis Long [:travis_]]

This may be by design, I don't seem to see any information about sending or not sending the ping depending on the setMetricsEnabled() state.  What I do see happening is that we are not recording any metrics if metrics are disabled, but I don't know if this affects the baseline ping because of it's lifetime attribute.  I'm pinging Alessio to get more info.

Comment 2

[Alessio Placitelli [:Dexter]]

(In reply to Travis Long from comment #1)
> This may be by design, I don't seem to see any information about sending or
> not sending the ping depending on the setMetricsEnabled() state.  What I do
> see happening is that we are not recording any metrics if metrics are
> disabled, but I don't know if this affects the baseline ping because of it's
> lifetime attribute.  I'm pinging Alessio to get more info.

Yes. Let's introduce a way to disable upload. This might be useful to implement the solution to the problem illustrated in bug 1514336 comment 1 as well.

Comment 3

[Travis Long [:travis_]]

Okay, by "persistently" (from the comment in bug 1514336), do we want it to be so that the glean lib manages and maintains that persistence?  Since we are only a library, it seems like persistently disabling/enabling sending of pings to be the responsibility of the caller.  I think having the setMetricsEnabled() flag control ping uploading is a good idea, but I'm not sure it's such a good idea to incorporate managing the persistence of that in the library itself.

Comment 4

[Alessio Placitelli [:Dexter]]

(In reply to Travis Long from comment #3)
> Okay, by "persistently" (from the comment in bug 1514336), do we want it to
> be so that the glean lib manages and maintains that persistence?  Since we
> are only a library, it seems like persistently disabling/enabling sending of
> pings to be the responsibility of the caller. 

I share the same concern about persistence, I think that should be up to the application to define. However, let's defer/take the persistence specific part to bug 1514336.

> I think having the
> setMetricsEnabled() flag control ping uploading is a good idea, but I'm not
> sure it's such a good idea to incorporate managing the persistence of that
> in the library itself.

I agree: let's have this flag control everything. We can always change it back later if we need to separate the semantics between controlling recording and uploading.

Comment 5

[Travis Long [:travis_]]

Attachment: GitHub Pull Request

Comment 6

[Travis Long [:travis_]]

Ping uploads now properly obey the prime directive (and the setMetricsEnabled() flag)