diff options
Diffstat (limited to 'toolkit/components/telemetry/docs/internals')
7 files changed, 776 insertions, 0 deletions
diff --git a/toolkit/components/telemetry/docs/internals/index.rst b/toolkit/components/telemetry/docs/internals/index.rst new file mode 100644 index 0000000000..78679bda0d --- /dev/null +++ b/toolkit/components/telemetry/docs/internals/index.rst @@ -0,0 +1,25 @@ +========= +Internals +========= + +Here is a quick overview of the most important code parts. They can be found in the telemetry folder. + +* TelemetryController: Main telemetry logic, e.g. assembling pings, local storage (when archiving is enabled), preference changes, testing +* Telemetry.cpp contains most of the public interface, implements the IDL +* The different data types for telemetry are handled in TelemetryHistogram, TelemetryScalar, TelemetryEvent. +* TelemetryEnvironment: A helper for gathering environment data, like build version or graphics data +* TelemetryScheduler: Starts regular jobs for collecting and sending data +* TelemetrySend: Sending and caching of pings +* TelemetryStorage: Handles writing pings to disk for TelemetrySend +* TelemetrySession: Collects data for a browsing session, includes many of the most important probes (aka metrics) +* Policy: A layer of indirection added to provide testability. A common pattern in many files +* pings/: Contains definitions and handling for most ping types, like EventPing + +More details on different topics can be found in these chapters: + +.. toctree:: + :maxdepth: 2 + :titlesonly: + :glob: + + ** diff --git a/toolkit/components/telemetry/docs/internals/integration_tests/index.rst b/toolkit/components/telemetry/docs/internals/integration_tests/index.rst new file mode 100644 index 0000000000..389fa8b80e --- /dev/null +++ b/toolkit/components/telemetry/docs/internals/integration_tests/index.rst @@ -0,0 +1,143 @@ +================= +Integration Tests +================= + +The aim of the telemetry-tests-client suite is to verify Firefox collects telemetry probes, aggregates that data, and submits telemetry +pings containing the data to a HTTP server. The integration tests try to make no assumptions about the internal workings of Firefox and +use automation to mimic user behavior. + +The integration test suite for Firefox Client Telemetry runs on CI `tier 1 <https://wiki.mozilla.org/Sheriffing/Job_Visibility_Policy>`_ +with treeherder symbol `tt(c)` +and is checked in to version control at mozilla-central under +`toolkit/components/telemetry/tests/marionette/tests/client <https://searchfox.org/mozilla-central/source/toolkit/components/telemetry/tests/marionette/tests/client/>`_. + +Test Main Tab Scalars +--------------------- + +- PATH: ``telemetry/tests/marionette/tests/client/test_main_tab_scalars.py`` +- This test opens and closes a number of browser tabs, + restarts the browser in a new session + and then verifies the correctness of scalar data in the resulting `main` ping. + +Test Search Counts +------------------ + +- PATH: ``toolkit/telemetry/tests/marionette/tests/client/test_search_counts_across_subsessions.py`` +- This test performs a search in a new tab, + restarts Firefox in a new session and verifies the correctness of client, session and subsession IDs, + as well as scalar and keyed histogram data in the `shutdown` ping, + installs an addon, verifies the `environment-change` ping, and performs three additional search actions + before restarting and verifying the new `main` ping. + + +Test Deletion Request Ping +-------------------------- + +- PATH: ``toolkit/telemetry/tests/marionette/tests/client/test_deletion_request_ping.py`` +- This test installs an addon and verifies a ping is received. The test takes note of the client ID. + It then disables telemetry and checks for a `deletion-request` ping. + After it receives the correct ping it makes sure that no other pings are sent. + Telemetry is then re-enabled and the `main` ping is checked to see if the client ID has changed. + The test asserts that the user has opted back in to telemetry. + +Test Event Ping +--------------- + +- PATH: ``toolkit/telemetry/tests/marionette/tests/client/test_event_ping.py`` +- This test checks for a basic `event` ping. It opens firefox, performs a search and checks the `event` + ping for the correct number of searches performed (1) and the correct search engine. + +Test Fog Custom Ping +-------------------- + +- PATH: ``toolkit/telemetry/tests/marionette/tests/client/test_fog_custom_ping.py`` +- This test creates a custom ping using the Glean API and asserts this ping is sent correctly. + +Test Fog Deletion Request Ping +------------------------------ + +- PATH: ``toolkit/telemetry/tests/marionette/tests/client/test_fog_deletion_request_ping.py`` +- This test opens the browser, performs a search and disables telemetry after the search. + It asserts that the telemetry is disabled and no pings exist. + The browser is restarted and telemetry is then re-enabled. + Then we set a `debug tag <https://mozilla.github.io/glean/book/user/debugging/debug-ping-view.html>`_ + which is attached to the ping. + Telemetry is then disabled again to trigger a `deletion-request` ping. + We verify that 1) The debug tag is present; and 2) that the client ID + in the second `deletion-request` ping is different from the first client ID. + +Test Fog User Activity +---------------------- + +- PATH: ``toolkit/telemetry/tests/marionette/tests/client/test_fog_user_activity.py`` +- This test checks that a `baseline` ping is sent when the user starts or stops using Firefox. + +Test Background Update Ping +--------------------------- + +- PATH: ``toolkit/telemetry/tests/marionette/tests/client/test_fog_user_activity.py`` +- In this test we launch Firefox to prepare a profile and to disable the background update setting. + We exit Firefox, + leaving the (unlocked) profile to be used as the default profile for the background update task (and not having multiple instances running). + The task will not try to update, but it will send a ping. + Then we restart Firefox to unwind the background update setting and allow shutdown to proceed cleanly. + +Running the tests locally +------------------------- + +You can run the tests on your local machine using +`mach <https://firefox-source-docs.mozilla.org/mach/index.html>`__: + +``./mach telemetry-tests-client`` + +Running the tests on try +------------------------ + +You can run the tests across all platforms on the try server using +`mach <https://firefox-source-docs.mozilla.org/mach/index.html>`__: + +``./mach try fuzzy -q "'telemetry-tests-client"`` + +Disabling an individual failing test +------------------------------------ + +The telemetry-tests-client suite is implemented in Python and uses Marionette for browser automation and wptserve for the HTTP ping server. +The integration tests are based on Python's unittest testing library and can be disabled by calling +`self.skipTest("reason") <https://docs.python.org/3/library/unittest.html#skipping-tests-and-expected-failures>`_ in a test method. + +The example below demonstrates how to disable test_main_ping2: + +.. code-block:: python + + import unittest + + from telemetry_harness.testcase import TelemetryTestCase + + class TestMainPingExample(TelemetryTestCase): + """Example tests for the telemetry main ping.""" + + def test_main_ping1(self): + """Example test that we want to run.""" + + self.search_in_new_tab("mozilla firefox") + + def test_main_ping2(self): + """Example test that we want to skip.""" + + self.skipTest("demonstrating skipping") + + self.search_in_new_tab("firefox telemetry") + + +Who to contact for help +----------------------- + +- The test cases are owned by Chris Hutten-Czapski (chutten on matrix) from the Firefox Telemetry team + (`#telemetry <https://chat.mozilla.org/#/room/#telemetry:mozilla.org>`__ on matrix) +- The test harness is owned by Raphael Pierzina (raphael on matrix) from the Ecosystem Test Engineering team + (`#telemetry <https://chat.mozilla.org/#/room/#telemetry:mozilla.org>`__ on matrix) + +Bugzilla +-------- + +Bugs can be filed under the Toolkit product for the Telemetry component. diff --git a/toolkit/components/telemetry/docs/internals/mentored-bugs.rst b/toolkit/components/telemetry/docs/internals/mentored-bugs.rst new file mode 100644 index 0000000000..bdc2de96b2 --- /dev/null +++ b/toolkit/components/telemetry/docs/internals/mentored-bugs.rst @@ -0,0 +1,49 @@ +========================== +Template for Mentored Bugs +========================== + +We like to encourage external contributions to the Firefox code base and the Telemetry module specifically. +In order to set up a mentored bug, you can use the following template. +Post it as a comment and add relevant steps in part 3. + +.. code-block:: md + + To help Mozilla out with this bug, here's the steps: + + 1. Comment here on the bug that you want to volunteer to help. + This will tell others that you're working on the next steps. + 2. [Download and build the Firefox source code](https://firefox-source-docs.mozilla.org/setup/index.html) + * If you have any problems, please ask on + [Element/Matrix](https://chat.mozilla.org/#/room/#introduction:mozilla.org) + in the `#introduction` channel. They're there to help you get started. + * You can also read the + [Firefox Contributors' Quick Reference](https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html), + which has answers to most development questions. + 3. Start working on this bug. <SPECIFIC STEPS RELEVANT TO THIS BUG> + * If you have any problems with this bug, + please comment on this bug and set the needinfo flag for me. + Also, you can find me and my teammates on the `#telemetry` channel on + [Element/Matrix](https://chat.mozilla.org/#/room/#telemetry:mozilla.org) + most hours of most days. + 4. Build your change with `mach build` and test your change with + `mach test toolkit/components/telemetry/tests/`. + Also check your changes for adherence to our style guidelines by using `mach lint` + 5. Submit the patch (including an automated test, if applicable) for review. + Mark me as a reviewer so I'll get an email to come look at your code. + * [Getting your code reviewed](https://firefox-source-docs.mozilla.org/setup/contributing_code.html#getting-your-code-reviewed) + * This is when the bug will be assigned to you. + 6. After a series of reviews and changes to your patch, + I'll mark it for checkin or push it to autoland. + Your code will soon be shipping to Firefox users worldwide! + 7. ...now you get to think about what kind of bug you'd like to work on next. + Let me know what you're interested in and I can help you find your next contribution. + + +Don't forget to add yourself as a :code:`Mentor` on the bug, +and add these tags to the :code:`Whiteboard`: + +* Add :code:`[lang=<language>]` to show what languages solving this bug will involve. +* Add one of :code:`[good first bug]`, :code:`[good second bug]`, :code:`[good next bug]` + to indicate for whom this bug might be a good point for contribution. + +If this is a Good First Bug, be sure to also add the :code:`good-first-bug` :code:`Keyword`. diff --git a/toolkit/components/telemetry/docs/internals/pingsender.rst b/toolkit/components/telemetry/docs/internals/pingsender.rst new file mode 100644 index 0000000000..be60f699ff --- /dev/null +++ b/toolkit/components/telemetry/docs/internals/pingsender.rst @@ -0,0 +1,36 @@ +Ping Sender +=========== + +The ping sender is a minimalistic program whose sole purpose is to deliver a +telemetry ping. It accepts the following parameters: + +- the URL the ping will be sent to (as an HTTP POST command); +- the path to an uncompressed file holding the ping contents. + +Once the ping has been read from disk the ping sender will try to post it once, exiting +with a non-zero value if it fails. If sending the ping succeeds then the ping file is removed. + +The content of the HTTP request is *gzip* encoded. The request comes with a few +additional headers: + +- ``User-Agent: pingsender/1.0`` +- ``X-PingSender-Version: 1.0``. Even if this data is already included by the user agent, this + header is needed as the pipeline is not currently storing use agent strings and doing that + could require storing a fair chunk of redundant extra data. We need to discern between pings + sent using the ping sender and the ones sent using the normal flow, at the end of the + ingestion pipeline, for validation purposes. + +.. note:: + + The ping sender relies on libcurl for Linux and Mac build and on WinInet for + Windows ones for its HTTP functionality. It currently ignores Firefox or the + system proxy configuration. + +In non-debug mode the ping sender doesn't print anything, not even on error, +this is done deliberately to prevent startling the user on architectures such +as Windows that would open a separate console window just to display the +program output. If you need runtime information to be printed out compile the +ping sender with debugging enabled. + +The pingsender is not supported on Firefox for Android +(see `bug 1335917 <https://bugzilla.mozilla.org/show_bug.cgi?id=1335917>`_) diff --git a/toolkit/components/telemetry/docs/internals/preferences.rst b/toolkit/components/telemetry/docs/internals/preferences.rst new file mode 100644 index 0000000000..0c63ae031d --- /dev/null +++ b/toolkit/components/telemetry/docs/internals/preferences.rst @@ -0,0 +1,280 @@ +Preferences and Defines +======================= + +Telemetry behaviour is controlled through the mozconfig defines and preferences listed here. + +mozconfig Defines +----------------- + +``MOZ_TELEMETRY_REPORTING`` + + When Defined (which it is for official builds): + + * If ``RELEASE_OR_BETA`` is not defined, defines ``MOZ_TELEMETRY_ON_BY_DEFAULT`` + + When Not Defined: + + * If ``datareporting.healthreport.uploadEnabled`` is locked, we print a message in the Privacy settings that you cannot turn on data submission and disabled the checkbox so you don't try. + * Android: hides the data submission UI to prevent users from thinking they can turn it on + * Disables Telemetry from being sent (due to ``Telemetry::IsOfficialTelemetry``) + +``MOZ_TELEMETRY_ON_BY_DEFAULT`` + + When Defined: + + * Android: enables ``toolkit.telemetry.enabled`` + +``MOZ_SERVICES_HEALTHREPORT`` + + When Defined (which it is on most platforms): + + * Sets ``datareporting.healthreport.{infoURL|uploadEnabled}`` in ``modules/libpref/init/all.js``. + +``MOZ_DATA_REPORTING`` + + When Defined (which it is when ``MOZ_TELEMETRY_REPORTING``, ``MOZ_SERVICES_HEALTHREPORT``, or ``MOZ_CRASHREPORTER`` is defined (so, on most platforms, but not typically on developer builds)): + + * Enables ``app.shield.optoutstudies.enabled`` + + When Not Defined: + + * Disables ``app.shield.optoutstudies.enabled`` + * Removes parts of the Data Collection Preferences UI in ``privacy.xhtml`` + +``MOZILLA_OFFICIAL`` + + When Not Defined (defined on our own external builds and builds from several Linux distros, but not typically on defeloper builds): + + * Disables Telemetry from being sent (due to ``Telemetry::IsOfficialTelemetry``) + +``MOZ_UPDATE_CHANNEL`` + + When not ``release`` or ``beta``: + + * If ``MOZ_TELEMETRY_REPORTING`` is also defined, defines ``MOZ_TELEMETRY_ON_BY_DEFAULT`` + + When ``beta``: + + * If ``toolkit.telemetry.enabled`` is otherwise unset at startup, ``toolkit.telemetry.enabled`` is defaulted to ``true`` (this is irrespective of ``MOZ_TELEMETRY_REPORTING``) + + When ``nightly`` or ``aurora`` or ``beta`` or ``default``: + + * Desktop: Locks ``toolkit.telemetry.enabled`` to ``true``. All other values for ``MOZ_UPDATE_CHANNEL`` on Desktop locks ``toolkit.telemetry.enabled`` to ``false``. + * Desktop: Defaults ``Telemetry::CanRecordExtended`` (and, thus ``Telemetry::CanRecordReleaseData``) to ``true``. All other values of ``MOZ_UPDATE_CHANNEL`` on Desktop defaults these to ``false``. + +``DEBUG`` + + When Defined: + + * Disables Telemetry from being sent (due to ``Telemetry::IsOfficialTelemetry``) + +**In Short:** + + For builds downloaded from mozilla.com ``MOZ_TELEMETRY_REPORTING`` is defined, ``MOZ_TELEMETRY_ON_BY_DEFAULT`` is on if you downloaded Nightly or Developer Edition, ``MOZ_SERVICES_HEALTHREPORT`` is defined, ``MOZ_DATA_REPORTING`` is defined, ``MOZILLA_OFFICIAL`` is defined, ``MOZ_UPDATE_CHANNEL`` is set to the channel you downloaded, and ``DEBUG`` is false. This means Telemetry is, by default, collecting some amount of information and is sending it to Mozilla. + + For builds you make yourself with a blank mozconfig, ``MOZ_UPDATE_CHANNEL`` is set to ``default`` and everything else is undefined. This means Telemetry is, by default, collecting an extended amount of information but isn't sending it anywhere. + +Preferences +----------- + +``toolkit.telemetry.unified`` + + This controls whether unified behavior is enabled. If true: + + * Telemetry is always enabled and recording *base* data. + * Telemetry will send additional ``main`` pings. + + It defaults to ``true``, but is ``false`` on Android (Fennec) builds. + +``toolkit.telemetry.enabled`` + + If ``unified`` is off, this controls whether the Telemetry module is enabled. It can be set or unset via the `Preferences` dialog in Firefox for Android (Fennec). + If ``unified`` is on, this is locked to ``true`` if ``MOZ_UPDATE_CHANNEL`` is ``nightly`` or ``aurora`` or ``beta`` or ``default`` (which is the default value of ``MOZ_UPDATE_CHANNEL`` for developer builds). Otherwise it is locked to ``false``. This controls a diminishing number of things and is intended to be deprecated, and then removed. + +``datareporting.healthreport.uploadEnabled`` + + If ``unified`` is true, this controls whether we send Telemetry data. + If ``unified`` is false, we don't use this value. + +``toolkit.telemetry.archive.enabled`` + + Allow pings to be archived locally. This can only be enabled if ``unified`` is on. + +``toolkit.telemetry.server`` + + The server Telemetry pings are sent to. Change requires restart. + +``toolkit.telemetry.log.level`` + + This sets the Telemetry logging verbosity per ``Log.sys.mjs``. The available levels, in descending order of verbosity, are ``Trace``, ``Debug``, ``Config``, ``Info``, ``Warn``, ``Error`` and ``Fatal`` with the default being ``Warn``. + + By default logging goes only the console service. + +``toolkit.telemetry.log.dump`` + + Sets whether to dump Telemetry log messages to ``stdout`` too. + +``toolkit.telemetry.shutdownPingSender.enabled`` + + Allow the ``shutdown`` ping to be sent when the browser shuts down, from the second browsing session on, instead of the next restart, using the :doc:`ping sender <pingsender>`. + +``toolkit.telemetry.shutdownPingSender.enabledFirstSession`` + + Allow the ``shutdown`` ping to be sent using the :doc:`ping sender <pingsender>` from the first browsing session. + +``toolkit.telemetry.firstShutdownPing.enabled`` + + Allow a duplicate of the ``main`` shutdown ping from the first browsing session to be sent as a separate ``first-shutdown`` ping. + +``toolkit.telemetry.newProfilePing.enabled`` + + Enable the :doc:`../data/new-profile-ping` on new profiles. + +``toolkit.telemetry.newProfilePing.delay`` + + Controls the delay after which the :doc:`../data/new-profile-ping` is sent on new profiles. + +``toolkit.telemetry.updatePing.enabled`` + + Enable the :doc:`../data/update-ping` on browser updates. + +``toolkit.telemetry.eventping.minimumFrequency`` + + The minimum frequency at which an :doc:`../data/event-ping` will be sent. + Default is 60 (minutes). + +``toolkit.telemetry.eventping.maximumFrequency`` + + The maximum frequency at which an :doc:`../data/event-ping` will be sent. + Default is 10 (minutes). + +``toolkit.telemetry.overrideUpdateChannel`` + + Override the ``channel`` value that is reported via Telemetry. + This is useful for distinguishing different types of builds that otherwise still report as the same update channel. + +``toolkit.telemetry.ipcBatchTimeout`` + + How long, in milliseconds, we batch accumulations from child processes before + sending them to the parent process. + Default is 2000 (milliseconds). + +``toolkit.telemetry.pioneerId`` + + If a user has opted into the Pioneer program, this will contain their Pioneer ID. + +Data-choices notification +------------------------- + +``toolkit.telemetry.reportingpolicy.firstRun`` + + This preference is not present until the first run. After, its value is set to false. This is used to show the infobar with a more aggressive timeout if it wasn't shown yet. + +``datareporting.policy.firstRunURL`` + + If set, a browser tab will be opened on first run instead of the infobar. + +``datareporting.policy.dataSubmissionEnabled`` + + This is the data submission master kill switch. If disabled, no policy is shown or upload takes place, ever. + +``datareporting.policy.dataSubmissionPolicyNotifiedTime`` + + Records the date user was shown the policy. This preference is also used on Android. + +``datareporting.policy.dataSubmissionPolicyAcceptedVersion`` + + Records the version of the policy notified to the user. This preference is also used on Android. + +``datareporting.policy.dataSubmissionPolicyBypassNotification`` + + Used in tests, it allows to skip the notification check. + +``datareporting.policy.currentPolicyVersion`` + + Stores the current policy version, overrides the default value defined in TelemetryReportingPolicy.sys.mjs. + +``datareporting.policy.minimumPolicyVersion`` + + The minimum policy version that is accepted for the current policy. This can be set per channel. + +``datareporting.policy.minimumPolicyVersion.channel-NAME`` + + This is the only channel-specific version that we currently use for the minimum policy version. + +GeckoView +--------- + +``toolkit.telemetry.geckoview.streaming`` + + Whether the GeckoView mode we're running in is the variety that uses the :doc:`GeckoView Streaming Telemetry API <../internals/geckoview-streaming>` or not. + Defaults to false. + +``toolkit.telemetry.geckoview.batchDurationMS`` + + The duration in milliseconds over which :doc:`GeckoView Streaming Telemetry <../internals/geckoview-streaming>` will batch accumulations before passing it on to its delegate. + Defaults to 5000. + +``toolkit.telemetry.geckoview.maxBatchStalenessMS`` + + The maximum time (in milliseconds) between flushes of the + :doc:`GeckoView Streaming Telemetry <../internals/geckoview-streaming>` + batch to its delegate. + Defaults to 60000. + +Testing +------- + +The following prefs are for testing purpose only. + +``toolkit.telemetry.initDelay`` + + Delay before initializing telemetry (seconds). + +``toolkit.telemetry.minSubsessionLength`` + + Minimum length of a telemetry subsession and throttling time for common environment changes (seconds). + +``toolkit.telemetry.collectInterval`` + + Minimum interval between data collection (seconds). + +``toolkit.telemetry.scheduler.tickInterval`` + + Interval between scheduler ticks (seconds). + +``toolkit.telemetry.scheduler.idleTickInterval`` + + Interval between scheduler ticks when the user is idle (seconds). + +``toolkit.telemetry.idleTimeout`` + + Timeout until we decide whether a user is idle or not (seconds). + +``toolkit.telemetry.modulesPing.interval`` + + Interval between "modules" ping transmissions. + +``toolkit.telemetry.send.overrideOfficialCheck`` + + If true, allows sending pings on unofficial builds. Requires a restart. + +``toolkit.telemetry.testing.overridePreRelease`` + + If true, allows recording opt-in Telemetry on the Release channel. Requires a restart. + +``toolkit.telemetry.untrustedModulesPing.frequency`` + + Interval, in seconds, between "untrustedModules" ping transmissions. + +``toolkit.telemetry.healthping.enabled`` + + If false, sending health pings is disabled. Defaults to true. + +``toolkit.telemetry.testing.disableFuzzingDelay`` + + If true, ping sending is not delayed when sending between 0am and 1am local time. + +``toolkit.telemetry.testing.overrideProductsCheck`` + + If true, allow all probes to be recorded no matter what the current product is. diff --git a/toolkit/components/telemetry/docs/internals/review.rst b/toolkit/components/telemetry/docs/internals/review.rst new file mode 100644 index 0000000000..80a3bd57de --- /dev/null +++ b/toolkit/components/telemetry/docs/internals/review.rst @@ -0,0 +1,144 @@ +=========================== +Telemetry review guidelines +=========================== + +General guidelines for reviewing changes +======================================== + +These are the general principles we follow when reviewing changes. + +- *Be constructive.* Both reviewers and patch authors should be allies that aim to get the change landed together. +- *Consider the impact.* We deliver critical data that is processed and used by many systems and people. Any disruption should be planned and intentional. +- *Be diligent.* All changes should be tested under typical conditions. +- *Know your limits.* Defer to other peers or experts where sensible. + +Main considerations for any change +======================================== + +For any change, these are the fundamental questions that we always need satisfactory answers for. + +- Does this have a plan? + + - Is there a specific need to do this? + - Does this change need `a proposal <https://github.com/mozilla/Fx-Data-Planning/blob/master/process/ProposalProcess.md>`_ first? + - Do we need to announce this before we do this? (e.g. for `deprecations <https://github.com/mozilla/Fx-Data-Planning/blob/master/process/Deprecation.md>`_) + +- Does this involve the right people? + + - Does this change need input from... A data engineer? A data scientist? + - Does this change need data review? + +- Is this change complete? + + - Does this change have sufficient test coverage? + - Does this change have sufficient documentation? + +- Do we need follow-ups? + + - Do we need to file validation bugs? Or other follow-up bugs? + - Do we need to communicate this to our users? Or other groups? + +Additional considerations +========================= + +Besides the basic considerations above, these are additional detailed considerations that help with reviewing changes. + +Considerations for all changes +------------------------------ + +- Follow our standards and best practices. + + - Firefox Desktop: + + - :ref:`The Mozilla coding style <Coding style>` + - `The toolkit code review guidelines <https://wiki.mozilla.org/Toolkit/Code_Review>`_ + + - Mobile: + + - `Android/Kotlin code style <https://kotlinlang.org/docs/reference/coding-conventions.html>`_ + - `iOS/Swift code style <https://github.com/mozilla-mobile/firefox-ios/wiki/Swift-Style-Guides>`_ + +- Does this impact performance significantly?: + + - Don't delay application initialisation (unless absolutely necessary). + - Don't ever block on network requests. + - Make longer tasks async whenever feasible. + +- Does this affect products more broadly than expected? + + - Consider all our platforms: Windows, Mac, Linux, Android. + - Consider all our products: Firefox, Fennec, GeckoView, Glean. + +- Does this fall afoul of common architectural failures? + + - Prefer APIs that take non-String types unless writing a parser. + +- Sanity checking: + + - How does this behave in a release build? Have you tested this? + - Does this change contain test coverage? We require test coverage for every new code, changes and bug fixes. + +- Does this need documentation updates? + + - To the :ref:`in-tree docs <Telemetry>`? + - To the `firefox-data-docs <https://docs.telemetry.mozilla.org/>`_ (`repository <https://github.com/mozilla/firefox-data-docs>`_)? + - To the `glean documentation <https://github.com/mozilla-mobile/android-components/tree/master/components/service/glean>`_? + +- Following up: + + - Do we have a validation bug filed yet? + - Do all TODOs have follow-up bugs filed? + - Do we need to communicate this to our users? + + - `fx-data-dev <https://mail.mozilla.org/listinfo/fx-data-dev>`_ (Main Firefox data list) + - `firefox-dev <https://mail.mozilla.org/listinfo/firefox-dev>`_ (Firefox application developers) + - `dev-platform <https://lists.mozilla.org/listinfo/dev-platform>`_ (Gecko / platform developers) + - `mobile-firefox-dev <https://mail.mozilla.org/listinfo/mobile-firefox-dev>`_ (Mobile developers) + - fx-team (Firefox staff) + + - Do we need to communicate this to other groups? + + - Data engineering, data science, data stewards, ...? + +Consider the impact on others +----------------------------- + +- Could this change break upstream pipeline jobs? + + - Does this change the format of outgoing data in an unhandled way? + + - E.g. by adding, removing, or changing the type of a non-metric payload field. + + - Does this require ping schema updates? + - Does this break jobs that parse the registry files for metrics? (Scalars.yaml, metrics.yaml, etc.) + +- Do we need to involve others? + + - Changes to data formats, ping contents, ping semantics etc. require involving a data engineer. + - Changes to any outgoing data that is in active use require involving the stakeholders (e.g. data scientists). + +Considerations for Firefox Desktop +---------------------------------- + +- For profiles: + + - How does using different profiles affect this? + - How does switching between profiles affect this? + - What happens when users switch between different channels? + +- Footguns: + + - Does this have side-effects on Fennec? (Unified Telemetry is off there, so behavior is pretty different.) + - Is your code gated on prefs, build info, channels? Tests should cover that. + - If test is gated on isUnified, code should be too (and vice-versa) + + - Test for the other case + + - Any code using `new Date()` should get additional scrutiny + - Code using `new Date()` should be using Policy so it can be mocked + - Tests using `new Date()` should use specific dates, not the current one + + - How does this impact Build Faster support/Artifact builds/Dynamic builtin scalars or events? Will this be testable by others on artifact builds? + - We work in the open: Does the change include words that might scare end users? + - How does this handle client id resets? + - How does this handle users opting out of data collection? diff --git a/toolkit/components/telemetry/docs/internals/tests.rst b/toolkit/components/telemetry/docs/internals/tests.rst new file mode 100644 index 0000000000..58adbb94af --- /dev/null +++ b/toolkit/components/telemetry/docs/internals/tests.rst @@ -0,0 +1,99 @@ +Tests +===== + +A high-level test strategy for Firefox Telemetry is defined in the +`Test Strategy document <https://docs.google.com/document/d/1Mi6va3gE4HSv5LjXNREvMa2V4q-LKIFDTwA2o4yeo_c/edit>`_. + +Firefox Telemetry is a complicated and old component. +So too are the organization and expanse of its tests. +Let’s break them down by harness. + +Unless otherwise mentioned the tests live in subdirectories of +``toolkit/components/telemetry/tests``. + +Mochitest +--------- +:Location: ``t/c/t/t/browser/`` +:Language: Javascript + (`mochitest <https://firefox-source-docs.mozilla.org/testing/mochitest-plain>`__) + +This test harness runs nearly the entire Firefox and gives access to multiple tabs and browser chrome APIs. +It requires window focus to complete correctly, +so it isn’t recommended to add new tests here. +The tests that are here maybe would be more at home as telemetry-tests-client tests as they tend to be integration tests. + +Google Test +----------- +:Location: ``t/c/t/t/gtest/`` +:Language: C++ + (`googletest <https://github.com/google/googletest>`_) + +This test harness runs a specially-built gtest shell around libxul which allows you to write unit tests against public C++ APIs. +It should be used to test the C++ API and core of Firefox Telemetry. +This is for tests like +“Do we correctly accumulate to bucket 0 if I pass -1 to ``Telemetry::Accumulate``?” + +Integration Tests (telemetry-tests-client and telemetry-integration-tests) +-------------------------------------------------------------------------- +:Location: ``t/c/t/t/marionette/tests/client`` and ``t/c/t/t/integration/`` +:Language: Python + (`unittest <https://docs.python.org/3/library/unittest.html>`__, + `pytest <https://docs.pytest.org/en/latest/>`_) + +The most modern of the test harnesses, +telemetry-integration-tests uses marionette to puppet the entire browser allowing us to write integration tests that include ping servers and multiple browser runs. +You should use this if you’re testing Big Picture things like +“Does Firefox resend its “deletion-request” ping if the network is down when Telemetry is first disabled?”. + +At time of writing there are two “editions” of integration tests. +Prefer writing new tests in telemetry-tests-client +(the unittest-based one in ``t/c/t/t/marionette/tests/client``) +while we evaluate CI support for telemetry-integration-tests. + +More info: :doc:`./integration_tests/index` + +Definitions Files Tests +----------------------- +:Location: ``t/c/t/t/python`` +:Language: Python + (`unittest <https://docs.python.org/3/library/unittest.html>`__) + +This harness pulls in the parsers and scripts used to turn JSON and YAML probe definitions into code. +It should be used to test the build scripts and formats of the definitions files +Histograms.json, Scalars.yaml, and Events.yaml. +This is for tests like +“Does the build fail if someone forgot to put in a bug number for a new Histogram?”. + +xpcshell +-------- +:Location: ``t/c/t/t/unit`` +:Language: Javascript + (`xpcshell <https://firefox-source-docs.mozilla.org/testing/xpcshell>`__) + +This test harness uses a stripped-down shell of the Firefox browser to run privileged Javascript. +It should be used to write unit tests for the Javascript API and app-level logic of Firefox Telemetry. +This is for tests like +“Do we correctly accumulate to bucket 0 if I pass -1 to ``Telemetry.getHistogramById(...).add``?” +and +“Do we reschedule pings that want to be sent near local midnight?”. + +Since these tests are easy to write and quick to run we have in the past bent this harness in a few interesting shapes +(see PingServer) +to have it support integration tests as well. +New integration tests should use telemetry-tests-client instead. + +Instrumentation Tests +--------------------- +:Location: Various +:Language: Usually Javascript + (`xpcshell <https://firefox-source-docs.mozilla.org/testing/xpcshell>`__ or + `mochitest <https://firefox-source-docs.mozilla.org/testing/mochitest-plain>`__) + +In addition to the tests of Firefox Telemetry, +other code owners have written tests that ensure that their code records appropriate values to Telemetry. +They should use the +``toolkit/components/telemetry/tests/unit/TelemetryTestUtils.sys.mjs`` +module to make their lives easier. +This can be used for tests like +“If five bookmarks are read from the database, +does the bookmark count Histogram have a value of 5 in it?”. |