1556057 - Develop an integration test for addons that register dynamic probes

Status: RESOLVED FIXED

(Toolkit :: Telemetry, enhancement, P3)

Description

[Raphael Aurich [:raphael] UTC+02:00]

Extend the telemetry-tests-client suite with a test focused on verifying that we can install addons that register dynamic probes and record to them.

Comment 1

[Benjamin Forehand Jr[:b4hand]]

What type of addon would register a dynamic probe? Could I get some more information on what that means, and how I can test it? TIA

Comment 2

[Chris H-C :chutten]

Mozilla-privileged addons have access to these APIs which include registerScalars and registerEvents. Think of an addon that adds a new feature (like Multi-Account Containers) -- it may want to instrument the feature it has added (to measure engagement, performance, stability, etc). There is no instrumentation in the browser and no probes in its Scalars.yaml, so the addon needs to describe the data collections it might want to accumulate data to.

Comment 3

[Benjamin Forehand Jr[:b4hand]]

Will this be tested against all versions of Firefox? Versions being Nightly, Beta, etc. Also, do we know of any current Mozilla extensions that use dynamic probes? I am wondering if we can just use one of them instead of rolling our own.

Comment 4

[Chris H-C :chutten]

A quick scan of dynamic process probes shows Normandy as a big user: https://sql.telemetry.mozilla.org/queries/75553/source

As for all versions: yes. To my knowledge, at present tt(c) runs on all releases. If it simplifies things, we can filter things down to only specific channels to start, and then widen to all channels as a follow-up.

Comment 5

[Benjamin Forehand Jr[:b4hand]]

Does this look good enough for an extension? I have tested it and it works but before I add it to the mozilla-extensions github org, I would like your thoughts.

browser.telemetry.registerScalars("dynamic.probe", {
    "counter_scalar": {
        kind: "count",
        keyed: false,
        record_on_release: false
    },
});
browser.telemetry.scalarSet("dynamic.probe.counter_scalar", 1337);

Comment 6

[Chris H-C :chutten]

You might want record_on_release: true to ensure we can run it on all channels, not just "nightly" and "beta", but otherwise it looks good to me!

Comment 7

[Benjamin Forehand Jr[:b4hand]]

Adding the extension https://github.com/mozilla-extensions/xpi-manifest/pull/81, will allow it to be built on taskcluster with elevated permissions

Comment 8

[Shane Caraveo (:mixedpuppy)]

Could someone explain what this adds in terms of testing that is not already covered in: https://searchfox.org/mozilla-central/source/toolkit/components/extensions/test/xpcshell/test_ext_telemetry.js

Comment 9

[Benjamin Forehand Jr[:b4hand]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #8)

Could someone explain what this adds in terms of testing that is not already covered in: https://searchfox.org/mozilla-central/source/toolkit/components/extensions/test/xpcshell/test_ext_telemetry.js

Hi Shane, sorry for the delayed response.

This test is a Marionette test, and is more of an e2e test than test_ext_telemetry.js is. The idea here is to test a real workflow, one that users will use, to make sure that there are no hangups along the way. Verifying that the entire telemetry payload, as well as the correct events, are sent to the telemetry server is what we are after for this test.

It is probably a good idea to reference test_ext_telemetry.js as a comment in this test though.

Comment 10

[Benjamin Forehand Jr[:b4hand]]

Hi Shane, I would like to know your thoughts on the above. Thanks.

Comment 11

[Shane Caraveo (:mixedpuppy)]

If an end to end test is desired that is fine. if it runs against a live (test) telemetry server that may be a bonus. If it doesn't, I think the current tests cover installing an addon and registering dynamic probes.

Comment 12

[Benjamin Forehand Jr[:b4hand]]

Attachment: Bug 1556057 - Develop an integration test for addons that register dynamic probes; r?raphael,chutten

Comment 13

[Benjamin Forehand Jr[:b4hand]]

As discussed on slack/zoom, I am running into some issues with this test. Here is what I have tried, all of which are unsuccessful.

1. I have tried to check the main ping for the scalar by restarting the browser after the addon is installed.
2. Add a delay after the addon installation
3. Restart the browser multiple times and check for the scalar to appear in the ping.

I am wondering if the extension itself needs some improvements to the way it sends the dynamic probe but I am not sure.

Comment 14

[Raphael Aurich [:raphael] UTC+02:00]

I added my comments to the Phabricator Diff. Please investigate whether installing the signed addon with temp=False solves the issue with restarting the browser.

Clearing the ni request for :chutten here while you look into this.

Comment 15

[Benjamin Forehand Jr[:b4hand]]

(In reply to Raphael Pierzina [:raphael] UTC+02:00 from comment #14)

I added my comments to the Phabricator Diff. Please investigate whether installing the signed addon with temp=False solves the issue with restarting the browser.

Clearing the ni request for :chutten here while you look into this.

This looks to have fixed it. Do we even need the temp flag for this suite?

Comment 16

[Benjamin Forehand Jr[:b4hand]]

Looks like temp is needed for the other unsigned addon.

Comment 17

[Raphael Aurich [:raphael] UTC+02:00]

(In reply to Benjamin Forehand Jr[:b4hand] from comment #16)

Looks like temp is needed for the other unsigned addon.

Yes, exactly. We need to add temp as a keyword argument to the internal install addon method.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a6861c17708e
Develop an integration test for addons that register dynamic probes;  r=raphael,chutten

Comment 19

[Natalia Csoregi [:nataliaCs]]

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