997820 - tests randomly connect to incoming.telemetry.mozilla.org

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Description

[Nathan Froyd [:froydnj]]

Despite toolkit.telemetry.enabled not being set, which should mean that we've disabled telemetry:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/TelemetryPing.jsm#891

We still see tests connecting to incoming.telemetry.mozilla.org:

https://tbpl.mozilla.org/php/getParsedLog.php?id=38017029&tree=Try
https://tbpl.mozilla.org/php/getParsedLog.php?id=38016931&tree=Try
https://tbpl.mozilla.org/php/getParsedLog.php?id=38016918&tree=Try

We need to fix this to enable non-local connection protection in the testsuites.  We also need to fix this as incoming telemetry data from tests might be seriously skewing results.

Comment 1

[Nathan Froyd [:froydnj]]

OK, so this is because of a number of things:

- Our try builds compile with --enable-update-channel=

- We have code to set MOZ_TELEMETRY_ON_BY_DEFAULT in configure.in:

if test "$MOZ_TELEMETRY_REPORTING"; then
    AC_DEFINE(MOZ_TELEMETRY_REPORTING)

    # Enable Telemetry by default for nightly and aurora channels
    if test -z "$RELEASE_BUILD"; then
      AC_DEFINE(MOZ_TELEMETRY_ON_BY_DEFAULT)
    fi
fi

Notice that decision is made by looking at RELEASE_BUILD, *not* the explicit update channel.

- RELEASE_BUILD is set using some completely different mechanism that's also not consulting the update channel:

# set RELEASE_BUILD and NIGHTLY_BUILD variables depending on the cycle we're in
# The logic works like this:
# - if we have "a1" in GRE_MILESTONE, we're building Nightly (define NIGHTLY_BUILD)
# - otherwise, if we have "a" in GRE_MILESTONE, we're building Nightly or Aurora
# - otherwise, we're building Release/Beta (define RELEASE_BUILD)
case "$GRE_MILESTONE" in
  *a1*)
      NIGHTLY_BUILD=1
      AC_DEFINE(NIGHTLY_BUILD)
      ;;
  *a*)
      ;;
  *)
      RELEASE_BUILD=1
      AC_DEFINE(RELEASE_BUILD)
      ;;
esac
AC_SUBST(NIGHTLY_BUILD)
AC_SUBST(RELEASE_BUILD)

- Finally, we have this bit of code in Preferences.cpp that attempts to be helpful:

  // Set up the correct default for toolkit.telemetry.enabled.
  // If this build has MOZ_TELEMETRY_ON_BY_DEFAULT *or* we're on the beta
  // channel, telemetry is on by default, otherwise not. This is necessary
  // so that beta users who are testing final release builds don't flipflop
  // defaults.
  if (Preferences::GetDefaultType(kTelemetryPref) == PREF_INVALID) {
    bool prerelease = false;
#ifdef MOZ_TELEMETRY_ON_BY_DEFAULT
    prerelease = true;
#else
    if (Preferences::GetDefaultCString(kChannelPref).EqualsLiteral("beta")) {
      prerelease = true;
    }
#endif
    PREF_SetBoolPref(kTelemetryPref, prerelease, true);
  }

which flips on telemetry for our try builds and tests.

We have code in mozprofile to disable telemetry for created profiles, but either we're not using that or it gets overridden by something else that I haven't discovered yet.

Comment 2

[Nathan Froyd [:froydnj]]

Luckily the Preferences.cpp thing seems to be a recent change.

Comment 3

[Nathan Froyd [:froydnj]]

Attachment: disable telemetry for tests

I chose two different strategies for the different kinds of tests here.  I
disabled telemetry entirely for {crash,ref,jsref}tests because we're not
running a proxy server there.  For mochitests, it's ok to try connecting to a
server that's going to 404 us, so we can just change the server and leave it
enabled.  I suppose we could be consistent and disable telemetry everywhere...

Comment 4

[Nathan Froyd [:froydnj]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dd9b23471fc1

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/dd9b23471fc1

Comment 6

[Nathan Froyd [:froydnj]]

I can still see tests on try connecting to incoming.telemetry.mozilla.org, even with this patch in place.  So I'm not quite sure if this has totally stuck yet.

Comment 7

[Nathan Froyd [:froydnj]]

Attachment: part 1.1 - really disable telemetry during reftests

Apparently the preferences setting in reftest-preferences.js runs too late to
properly affect telemetry startup.  Therefore, we need to set the preference in
the profile itself so things get initialized properly.

Comment 8

[Nathan Froyd [:froydnj]]

Attachment: part 2 - disable telemetry in jsreftests

jsreftests uses a completely different framework, so we need to disable
telemetry there as well.

Comment 9

[Nathan Froyd [:froydnj]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4797c2555c1b
https://hg.mozilla.org/integration/mozilla-inbound/rev/9bb8fb4f73f8

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4797c2555c1b
https://hg.mozilla.org/mozilla-central/rev/9bb8fb4f73f8

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/226410438634
https://hg.mozilla.org/releases/mozilla-beta/rev/a7ab7384526e
https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/8cbc1c4cb960
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/b02758710f9d
https://hg.mozilla.org/releases/mozilla-esr24/rev/1ac04a08b2f6

Comment 12

[Ryan VanderMeulen [:RyanVM]]

And an esr24 follow-up from rebasing silliness on my part.
https://hg.mozilla.org/releases/mozilla-esr24/rev/2d09a4707dd3

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/a7ab7384526e