Chris H-C :chutten Reporter
	Description • 7 years ago

Latest Nightlies have UI to show when WebDriver (Marionette (Selenium)) is in control.

We have a little problem identifying sessions that are bot-controlled in Telemetry, so it'd be really nice if that information could be passed along to Telemetry.

It'd be especially nice if it were a pref, because then it'd be easy to tie into our existing environment-change ping and userPrefs environment block.

	Andreas Tolfsen ❲:ato❳
	Comment 1 • 7 years ago

I don’t understand what this bug is about.  What is the ‘problem’ you have identifying Firefox session where the remote protocol is active?

You can query the nsIMarionette XPCOM interface (https://searchfox.org/mozilla-central/source/testing/marionette/components/nsIMarionette.idl) to find out if Marionette is running, or check the "remotecontrol" attribute on any <xul:browser> element.

In extension of my above question, what is the relevance to Telemetry to measure when the remote protocol is being used?  When the remote protocol is enabled, Marionette actively instruments the browser preferences so that we do not submit data to Telemetry so that measurements from (very) slow CI environments do not negatively impact the overall Telemetry results.

(In reply to Chris H-C :chutten from comment #0)
> It'd be especially nice if it were a pref, because then it'd be easy to tie
> into our existing environment-change ping and userPrefs environment block.

If what were a preference?

	Chris H-C :chutten Reporter
	Comment 2 • 7 years ago

The problem is how we receive quite a lot of data from bots in testfarms and I supposed this was due to selenium-driven profiles that were sending Telemetry. Alessio had to introduce a certain amount of complexity into the new-profile ping to avoid sending pings from these profiles.

If these are coming from some other automation mechanism and not Selenium, this issue is moot.

	Andreas Tolfsen ❲:ato❳
	Comment 3 • 7 years ago

(In reply to Chris H-C :chutten from comment #2)
> The problem is how we receive quite a lot of data from bots in testfarms and
> I supposed this was due to selenium-driven profiles that were sending
> Telemetry. Alessio had to introduce a certain amount of complexity into the
> new-profile ping to avoid sending pings from these profiles.
> 
> If these are coming from some other automation mechanism and not Selenium,
> this issue is moot.

Thanks for clarifying!  Maybe I can shed some light on the situation.

Maybe it is the case that we are not disabling our Telemetry submission correctly.  

When Marionette is enabled in Firefox, either via Selenium or other clients, we set the preferences seen in https://searchfox.org/mozilla-central/rev/24c443a440104dabd9447608bd41b8766e8fc2f5/testing/marionette/server.js#48-265 if they have not been user-defined (modified) in the profile.

Many of these are being user-defined in https://github.com/mozilla/geckodriver/blob/master/src/prefs.rs if the client uses geckodriver, which normally is the case when using Selenium with recent versions of Firefox.  There are still a number of users on FirefoxDriver (not maintained by Mozilla) on Firefox 47 and earlier, and this does not try to prohibit submission to Telemetry.

	Chris H-C :chutten Reporter
	Comment 4 • 7 years ago

Alessio, do you know if https://searchfox.org/mozilla-central/rev/24c443a440104dabd9447608bd41b8766e8fc2f5/testing/marionette/server.js#163-171 is sufficient to shut things down? Or if you know if the testlabs in question manually set some of these?

	Georg Fritzsche [:gfritzsche]
	Comment 5 • 7 years ago

"datareporting.healthreport.uploadEnabled" is the main pref here, that is sufficient to stop uploading. [1]
We should not trust any 3rd parties to set any of those prefs though.

1: http://gecko.readthedocs.io/en/latest/toolkit/components/telemetry/telemetry/internals/preferences.html#id1

	Alessio Placitelli [:Dexter]
	Comment 6 • 7 years ago

Clearing the ni? as Georg already answered in comment 5.

	David Burns :automatedtester
	Comment 7 • 7 years ago

:chutten

What would be the best thing for us to do here seeing comment 5?

	Chris H-C :chutten Reporter
	Comment 8 • 7 years ago

The best thing to do is to just shut down this bug as invalid. Well-formed marionette cases will shut down upload, which is exactly what it should do. I guess any bots sending us bogus data are doing something else.

	Andreas Tolfsen ❲:ato❳
	Comment 9 • 7 years ago

Thanks for your feedback, chutten!  I have a small low-priority request to make.  Could you review the prefs we set in https://searchfox.org/mozilla-central/source/testing/geckodriver/src/prefs.rs and https://searchfox.org/mozilla-central/source/testing/marionette/server.js#53 related to the datareporting.* branch and tell us if we need all of them, or if a cleanup is in order?  Are there other prefs we should set?

	Chris H-C :chutten Reporter
	Comment 10 • 7 years ago

Redirecting to :gfritzsche who knows more about the prefs.

	Georg Fritzsche [:gfritzsche]
	Comment 11 • 7 years ago

Alessio, can you take a look?

	Alessio Placitelli [:Dexter]
	Comment 12 • 7 years ago

(In reply to Andreas Tolfsen ‹:ato› from comment #9)
> Thanks for your feedback, chutten!  I have a small low-priority request to
> make.  Could you review the prefs we set in
> https://searchfox.org/mozilla-central/source/testing/geckodriver/src/prefs.
> rs and
> https://searchfox.org/mozilla-central/source/testing/marionette/server.js#53
> related to the datareporting.* branch and tell us if we need all of them, or
> if a cleanup is in order?  Are there other prefs we should set?

Hey Andreas,

the currently valid prefs are documented at [1]. Let's go through the ones in the files you mentioned:

- "datareporting.healthreport.about.reportUrl" - This is needed so that opening about:telemetry doesn't hit the network
- "datareporting.healthreport.documentServerURI" - This was used before Telemetry V2 and it's not being used anymore [2].
- "datareporting.healthreport.logging.consoleEnabled" - This was used before Telemetry V2 and it's not being used anymore [2].
- "datareporting.healthreport.service.enabled" - This was used before Telemetry V2 and it's not being used anymore [2].
- "datareporting.healthreport.service.firstRun" - This was used before Telemetry V2 and it's not being used anymore [2].
- "datareporting.healthreport.uploadEnabled" - This is needed so that Telemetry doesn't try to hit the network to send pings.
- "datareporting.policy.dataSubmissionEnabled" - This has a meaning, but there's no need to set it: enabling it has the same effect of setting "uploadEnabled:false" and "dataSubmissionPolicyBypassNotification:true". You can safely leave this out and set the other prefs.
- "datareporting.policy.dataSubmissionPolicyAccepted" - There's no need to set this, it's only checked if "dataSubmissionPolicyBypassNotification:false". You can safely leave it out.
- "datareporting.policy.dataSubmissionPolicyBypassNotification" - This is needed to skip the "data choices" infobar notification in tests. You can set this to true.

[1] - https://gecko.readthedocs.io/en/latest/toolkit/components/telemetry/telemetry/internals/preferences.html
[2] - https://gecko.readthedocs.io/en/latest/toolkit/components/telemetry/telemetry/fhr/architecture.html