Closed Bug 1427877 Opened 2 years ago Closed 4 months ago

Telemetry from Thunderbird

Categories

(Toolkit :: Telemetry, task, P5)

task

Tracking

()

RESOLVED FIXED
mozilla71

People

(Reporter: firefox, Assigned: benc)

Details

Attachments

(3 files, 15 obsolete files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
14.91 KB, patch
mkmelin
: review+
Details | Diff | Splinter Review
Generally, just trying to find any Thunderbird data in Telemtry.

Specifically to help make the decision around removing flash support (https://bugzilla.mozilla.org/show_bug.cgi?id=1423363)

The two items that would be useful:
* how often it is installed
* how often it is used (or any kind of usage data from the Flash, if it's ever been run, etc)

Thanks!
Per IRC, bug 956101 (re-)enabled Telemetry for Thunderbird.

I assume that Thunderbird does not use Unified Telemetry (see [1]).
This means that:
(1) TB sends saved-session pings, not main pings. [2]
(2) TB sends no data from the release channel, only prerelease.
(3) TB only sends data on app shutdown.

For the telemetry.mozilla.org dashboard we only process main pings now, so TB data would not show up on it anymore.
Other internal data tools generally only process main pings, so we can't query incoming data in a convenient way.

For one-off questions, we could run custom jobs on the raw pings and get some data out.

We are about to remove saved-session pings this quarter or so in the Telemetry code base.
Is Telemetry in general something Thunderbird needs and can spend a smaller amount of time on to make it work?
If it was updated to use what Firefox uses now, we might be able to support it through the same infrastructure.

@Philipp: I see you listed at [3] - do you know who to ask these type of questions for TB?

1: https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/internals/preferences.html#id1
2: https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/data/main-ping.html
3: https://wiki.mozilla.org/Modules/All#Thunderbird_Council
Flags: needinfo?(philipp)
I'm always a good contact for TB things and will defer if necessary. For Telemetry, I believe Wayne is the person most interested. We would love to be able to use telemetry for Thunderbird, there are some things we'd like to be able to find out as well.

Tom will be available to help set up anything necessary on the build side, and if there is anything we need to include from Toolkit I'm sure we can get that done as well.

Specifically for Flash, I don't think there will be high usage and if it takes too long to set this up or gather the data I'd say it is ok to remove. Magnus would be a good person to make that call as the TB module owner.
Flags: needinfo?(vseerror)
Flags: needinfo?(philipp)
Flags: needinfo?(mkmelin+mozilla)
(In reply to Philipp Kewisch [:Fallen]  from comment #2)
> I'm always a good contact for TB things and will defer if necessary. For
> Telemetry, I believe Wayne is the person most interested. We would love to
> be able to use telemetry for Thunderbird, there are some things we'd like to
> be able to find out as well.

And having it as a working model at mozilla may help if we decide to stand up our own infrastructure.


(In reply to Georg Fritzsche [:gfritzsche] from comment #1)
> Per IRC, bug 956101 (re-)enabled Telemetry for Thunderbird.
> 
> I assume that Thunderbird does not use Unified Telemetry (see [1]).
> This means that:
> (1) TB sends saved-session pings, not main pings. [2]
> (2) TB sends no data from the release channel, only prerelease.
> (3) TB only sends data on app shutdown.

Correct, Thunderbird never adjusted to the FHR/telemetry changes, which I had hoped to jump start when I emailed to tb-planning on 3/8/2015 "FHR/Telemetry unification".  The unification was subsequently blogged about at http://robertovitillo.com/2015/06/27/a-glance-at-unified-fhrtelemetry/ and mentioned in Bug 1177121 Redesigning the crash experience

NOTE Thunderbird currently does near zero telemetry. But if we are to plan the future of Thunderbird without lots of guesswork and differing personal opinions about what we "think" users see then we really need data, and arguably lots of it.

----
As background, and for the record, per  https://mail.mozilla.org/pipermail/tb-planning/2011-October/001384.html
"The three probes we have are:
- THUNDERBIRD_GLODA_SIZE_MB : many people have a size of 1MB or less, which tends to make me think these people have Gloda disabled. I wonder how much of a correlation there is between being a tester and disabling Gloda; there's only one pathological user with a size that's greated than 1GB.
- THUNDERBIRD_CONVERSATIONS_TIME_TO_2ND_GLODA_QUERY : very few people reporting on that one, I think the Telemetry reporting code is not in the currently public version of Conversations, so I guess I need to make a new release
- THUNDERBIRD_INDEXING_RATE_MSG_PER_S : the distribution is pretty smooth, and the mean seems to be around 30 messages per sec, although there's an alarmingly high number of people indexing 100 messages per sec; maybe we need to make the scale bigger (do they have SSDs?)."
Flags: needinfo?(vseerror)
(In reply to Wayne Mery (:wsmwk) from comment #4)
> FWIW 
> - a Thunderbird telemetry link which still "works" is
> https://telemetry.mozilla.org/new-pipeline/dist.html#!
> cumulative=0&end_date=2015-11-05&keys=__none__!__none__!
> __none__&max_channel_version=nightly%252F45&measure=GC_MS&min_channel_version
> =null&processType=*&product=Thunderbird&sanitize=0&sort_keys=submissions&star
> t_date=2015-11-04&table=0&trim=1&use_submission_date=0
> - an example of unified firefox crash data
> https://telemetry.mozilla.org/crashes/

Pro Tip: the little link icon at the top right of telemetry.mozilla.org plots gives you a shortened link for easier sharing: https://mzl.la/2Fdjyeb

For the original topic, it seems as though there's a long road to use Telemetry in Thunderbird to make a Flash Go/NoGo decision. Step 1 would likely be adopting unified Telemetry (toolkit/components/telemetry/), Step 2 would be adding probes to measure Flash use, Step 3 would be shipping software, Step 4 would be analysis. None of these steps are easy :S
Indeed, that doesn't seem worth it for a Flash Go/NoGo (especially given Flash's EOL).  Instead I did an analysis of popular RSS feeds and found no flash usage.
This needs some conversations to happen first, to determine if this is needed.
Priority: -- → P5

Strange, I thought we had another bug filed for Thunderbird telemetry in general. Wayne, am I missing it? Regardless of this specific data point we'd like to be able to use telemetry in Thunderbird, we can either file a new bug in Thunderbird land or just morph this one since it already contains some info.

Looking at the internals link you mentioned earlier it seems that all we may be missing is setting datareporting.healthreport.uploadEnabled to true. We already have toolkit.telemetry.unified set to true.

I'm not sure how to differentiate saved-session pings from main pings from a code perspective, are there some functions that would send the one or the other? Would it be fair to assume that all code related to sending telemetry pings is in toolkit?

Flags: needinfo?(mkmelin+mozilla) → needinfo?(gfritzsche)

Georg's out on PTO, but I can answer some questions.

(In reply to Philipp Kewisch [:Fallen] [:📆] from comment #8)

Looking at the internals link you mentioned earlier it seems that all we may be missing is setting datareporting.healthreport.uploadEnabled to true. We already have toolkit.telemetry.unified set to true.

That might do the trick. There are some other policy-related things you'll need as well (ensuring your Privacy Notice covers the collections, allowing the user to view the Privacy Notice for some period of time before sending initial Telemetry (see TelemetryReportingPolicy.jsm), ensuring the user has UI to opt out of collections, ...), and there might be some interesting corner cases (I don't know if Thunderbird builds pingsender, for instance), but flipping the pref is certainly necessary if not sufficient.

I'm not sure how to differentiate saved-session pings from main pings from a code perspective, are there some functions that would send the one or the other? Would it be fair to assume that all code related to sending telemetry pings is in toolkit?

With toolkit.telemetry.unified set to true there are no "saved-session" pings any more, just "main" pings. (This is as of last April or so since bug 1443603).

All Firefox Desktop Telemetry ping assembly and sending code is in toolkit/components/telemetry. Custom pings ("uitour-tag", "sync", others) may live elsewhere, and underpinning tech (like the XHR we use to POST the ping and ipc we use to collate accumulations) lives wherever that stuff lives, but the central pings are there

Flags: needinfo?(gfritzsche)

This sounds great, thanks for the info! I seem to recall we've had a notification for telemetry before, so maybe it isn't very much work to revive and extend that. We'll also take a look to see if our privacy policy covers it. Magnus, can you prioritize this accordingly and have someone work on it?

(In reply to Philipp Kewisch [:Fallen] [:📆] from comment #8)

Strange, I thought we had another bug filed for Thunderbird telemetry in general. Wayne, am I missing it? Regardless of this specific data point we'd like to be able to use telemetry in Thunderbird, we can either file a new bug in Thunderbird land or just morph this one since it already contains some info.

I am not aware of any open bug reports. The most "recent" history of bugs is
fixed Bug 956060 - Fix missing raw Thunderbird telemetry data
fixed Bug 956101 - Thunderbird data is missing from telemetry.mozilla.org
https://github.com/mozilla/telemetry-dashboard/issues/16 Telemetry Dashboard no longer supports Thunderbird

Note, "support" for plugins was dropped in Thunderbird Bug 1508942 - Remove all support for plugins (applets, etc.)

I've had a dig through the telemetry system, and written up some notes. I haven't really looked at the server/analysis side of things - I've just been catching data locally and figuring out how to start adding telemetry probes to TB.

Here are my notes:

Telemetry in Thunderbird

Docs

The Telemetry doc index is at:

https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/index.html

There's a good summary of settings (both compiletime and runtime prefs):

https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/internals/preferences.html

Compiling

Telemetry is not compiled in by default. You need to add the following line
to your mozconfig:

export MOZ_TELEMETRY_REPORTING=1

The nightly and release configs have this setting already ($ grep -r MOZ_TELEMETRY_ mail/config/mozconfigs).

Prefs

There's a complex set of conditions to set up telemetry reporting.
The runtime settings needed for a minimal test setup are:

  • toolkit.telemetry.server - URL where the collected data will be POSTed to
    (https://incoming.telemetry.mozilla.org). So if you're running a local
    server for testing, you'll likely want this to be some localhost URL.
  • toolkit.telemetry.server.owner - The owner of the server (Mozilla).
    The implication is that it's polite to change this if you're running a
    non-Mozilla server.
  • toolkit.telemetry.send.overrideOfficialCheck - usually, telemetry is only
    send for official builds (ie export MOZILLA_OFFICIAL=1 in mozconfig).
    Setting this to true enables sending for unofficial builds.
  • datareporting.policy.dataSubmissionEnabled - allows submission to the
    server.
  • datareporting.policy.dataSubmissionPolicyBypassNotification - bypasses the
    checks to see if the policy has been shown and agreed to by the user. Set it
    to true for testing.
  • toolkit.telemetry.log.level - very handy for watching telemetry activity in the javascript console.Trace,Debug,Info,Warn`, etc...

eg paste into your prefs.js:

user_pref("toolkit.telemetry.server", "http://localhost:8080/wibble");
user_pref("toolkit.telemetry.server_owner", "Nobody");
user_pref("datareporting.policy.dataSubmissionPolicyBypassNotification",true);
user_pref("datareporting.policy.dataSubmissionEnabled", true);
user_pref("toolkit.telemetry.log.level", "Trace");
user_pref("toolkit.telemetry.send.overrideOfficialCheck", true);

TODO: need to check the default prefs included in builds to see if there
are any telemetry-related changes required (both for testing and for release
builds).

Data-collection Policy

It's expected that the user will have been shown and agreed to the data
collection policy. For testing we can bypass this via
datareporting.policy.dataSubmissionPolicyBypassNotification
but there are a bunch of settings which track which version of the policy
the user has agreed to.

I haven't looked into the UI side here. I'd guess a bit of work needs to be
done to show/update/record policy info, using the firefox UX as a template.

Debugging

Running a test server

To run a test server locally to dump out the sent data, try
https://github.com/mozilla/gzipServer
(or alternatively https://github.com/bcampbell/webhole).

Make sure you set toolkit.telemetry.server/toolkit.telemetry.server_owner
to point to your local server.

Log output

If you've got logging on (eg user_pref("toolkit.telemetry.log.level", "Trace");),
the output will show up on the javascript console:

Menu => "Tools" => "Developer Tools" => "Error Console"

If data isn't showing up, keep an eye out for messages in the console.
For example: "Telemetry is not allowed to send pings" is an indication that
the official-build check is failing (overridden by
toolkit.telemetry.send.overrideOfficialCheck).

Test pings

From the javascript console, you can force an immediate test ping:

Cu.import("resource://gre/modules/TelemetrySession.jsm");
TelemetrySession.testPing()

Adding a telemetry probe

The types of data collection are outlined here:

https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/start/adding-a-new-probe.html#rich-data-aggregate-data

The probe definitions are contained in .yaml and .json files under toolkit/components/telemetry (Scalars.yaml, Events.yaml etc).
Of course, these probes are all specific to Firefox.

The definitions are set at build time, using a bunch of python scripts in
toolkit/components/telemetry/build_scripts to generate the C++ files which
define the probe resistry (enums, string tables etc).

The code-generation scripts look like they can handle multiple probe definition files,
so we should be able to add extra .yaml/.json files into the mix with our new probes.
But currently the build process doesn't allow for invoking the build scripts with
multiple arguments, so we'd need at least a small M-C patch to properly support extra
Thunderbird-specific probes.

We'd need a way to hook toolkit/components/telemetry/moz.build to slip extra
files into the histogram_files, scalar_files etc... lists. Not quite sure
what the best way to do this is.

There may be some serverside considerations - if the probe definitions are used also
by the telemetry server (I'm unsure) we need to make sure that's accounted for on the
server setup.

Custom pings

For now, the most accessible way to add telemetry is to use a custom ping, which
doesn't require any build-time setup. See:

https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/collection/custom-pings.html

Other considerations

We probably need some coordination on what data is to be collected.
Perhaps a lightweight version of the
firefox process?

Do we need to worry about telemetry accidentally being sent out during
locally-run xpcshell tests? (I'd guess no. In the worst case it might
throw some rubbish at your local test server, but a default profile
won't be set up with all the policy acceptance prefs required to hit a
real live server).

Should we use a "tb-" or "mail-" prefix on TB-specific probe names to avoid
clashes?

Just a hacky little example of adding a custom ping (via submitExternalPing()). It's on the feeds code - whenever a feed is downloaded it pings out the feed type (feedType:rss/atom/etc) and the number of items found (nItems).

It'll send out something like this:

{"type":"tb-feed-parsed","id":"894174c5-93d2-4500-9a49-2518b4c36a66","creationDate":"2019-04-29T02:02:29.863Z","version":4,"application":{"architecture":"x86-64","buildId":"20190425081709","name":"Thunderbird","version":"68.0a1","displayVersion":"68.0a1","vendor":"","platformVersion":"68.0a1","xpcomAbi":"x86_64-gcc3","channel":"default"},"payload":{"feedType":"ATOM_IETF","nItems":10}}

I think the next thing to do is try and test adding some thunderbird-specific build-time-defined probes, and make sure they show up in a useful form in the analysis tools. I'm a little concerned about polluting the live server with accidental rubbish though...

Probably also a good time to start thinking up questions we'd like to see answered by telemetry. I think that should be what drives the metrics we choose to collect.

Assignee: nobody → benc

(In reply to Ben Campbell from comment #13)

Probably also a good time to start thinking up questions we'd like to see answered by telemetry. I think that should be what drives the metrics we choose to collect.

A hearty +1 to that. A questions-first approach often leads to the leanest data collection.

Speaking of which, Mozilla also published https://leandatapractices.org to help organizations construct their data collection practices. It might be of help.

Attached patch cc-add-tb-mbox-size-probe.patch (obsolete) — Splinter Review

This patch is a little test run of defining a new thunderbird-specific Scalar probe and getting it included into the telemetry code generation. This is the C-C part, and requires the corresponding M-C patch too.

Attached patch mc-add-tb-mbox-size-probe.patch (obsolete) — Splinter Review

... and here's the corresponding M-C patch. It's currently hardcoded to look for our custom defintions in comm/mail/components/telemetry/Scalars_tb.yaml, but I think some sort of CONFIG setting could work (see example in the patch) if I can work out where to set it.

Further thoughts:

  1. it looks like it's possible to slip in additional TB-specific probe definitions to the telemetry system (albeit with some changes to the M-C code generation, which we need to keep as unobtrusive as possible).

  2. Does the server/analysis dashboard side rely on the probe definitions in toolkits/components/telemetry? Seeing the description fields in https://telemetry.mozilla.org/probe-dictionary seems to imply that it does. How does the server get hold of these?

  3. adding extra probes is likely to alter the internal enum values in the C++ code. Does the server side ever rely on this numbering, or is it considered purely internal to the C++? (I can't imagine the numbering is important - it'll change all the time. I'd guess that it's only ever strings that go over the wire, but worth checking!)

Hey, another Telemetry team member here.

  1. Maybe we can work something out and make it more easy to allow additional files in m-c, so that the code bases don't need to diverge too much. I think we'd be ok with taking patches to make that happen
  2. The probe-scraper reads the same definition files from m-c and other places. If I'm not mistaken so far it's mostly an informational service (like exposing it in the probe-dictionary) and nothing for the analysis depends on that, in the future our backend might depend a bit more on the definitions
  3. Nope, enum values are purely internal, to the outside everything is identified by its name.

(In reply to Jan-Erik Rediger [:janerik] from comment #18)

Hey, another Telemetry team member here.

  1. Maybe we can work something out and make it more easy to allow additional files in m-c, so that the code bases don't need to diverge too much. I think we'd be ok with taking patches to make that happen

Great - I'm hoping to come up with something as trivial and unobtrusive as possible! (the M-C patch above being a rough first check to see if it's feasible to hook in extra definition files).

  1. The probe-scraper reads the same definition files from m-c and other places. If I'm not mistaken so far it's mostly an informational service (like exposing it in the probe-dictionary) and nothing for the analysis depends on that, in the future our backend might depend a bit more on the definitions

That's good to know. I think the idea would be to run a new instance of the existing backend, with as little change as possible.
(and I had an idea parse_scalars.py et al might show up again somewhere ;-) - they're in the M-C code-generation, with copies in probe-scraper, it looks like).

  1. Nope, enum values are purely internal, to the outside everything is identified by its name.

Wonderful, that all sounds very promising - thanks for jumping in!

One extra note is that the aggregator that powers telemetry.mozilla.org's Measurement Dashboard soaks the definitions files (Histograms.json, Scalars.yaml, Events.yaml) directly from hg.mozilla.org. Its successor (planned for later this year) will likely use the probe-scraper, but for now these paths are hardcoded there.

This should only matter if you were hoping to have the aggregator aggregate your metrics, too. If not, then having them be in a separate area and brought in at build time may actually be the preferable solution.

Would it help to organize a chat or meeting of some kind? We might have some helpful tips or near-future plans that could help out. Or at the very least we confirm things like how you'll definitely need to include all the m-c scalars, events, and histograms definitions (like you're doing already in your test patches) otherwise things'll start error-ing out pretty hard.

Sounds like we should indeed have a meeting! Ben, sancus (see cc) could attend from Thunderbird, and I'm interested as well (but timezone diff may it inconvenient). I think Ben is UTC+13, sancus UTC-7 and I'm UTC +3. Are you able to suggest a time?

Also if there is a main up-to-date documentation page about telemetry somewhere that could be useful reading for us.

Flags: needinfo?(chutten)

A meeting would be excellent. We've taken scheduling to email.

As for documentation, the in-tree docs might be the most relevant for the client portions: https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/index.html

This might be less relevant, but we also document the systems and datasets that the client-side collections support: https://docs.telemetry.mozilla.org/

You can find us on irc.mozilla.org (for now) at #telemetry if you'd like some more synchronous ways to chat.

Flags: needinfo?(chutten)

Just some notes on the probe parsers:

Bug 1282098 talks about moving them out of M-C... but I think the consensus was to leave the canonical source in M-C, and have a PyPi package which tracks it (for the data team, which need access to the probe definitions but won't necessarily have M-C).

probe-scraper has it's own copy of the probe parsers, with some changes that haven't yet been merged (see https://github.com/mozilla/probe-scraper/issues/6)

OK, so here's a more fully-formed attempt at patching M-C to handle extra probe definition files.

Firstly, it patches the various code-generation scripts so they can load/merge definitions from multiple files.

Secondly, it alters the telemetry moz.build to check config vars for extra probe definition files:
MOZ_TELEMETRY_EXTRA_SCALAR_FILES
MOZ_TELEMETRY_EXTRA_HISTOGRAM_FILES
MOZ_TELEMETRY_EXTRA_EVENT_FILES

(and those vars can be set in the top-level TB moz.configure using set_config())

I'm not totally happy with this approach, but it's the least-intrusive solution I've come up with. If anyone has a better solution, I'm all ears!
Failing that, I'll try and knock it into a proper proposal for applying to M-C. I suspect it might need breaking up - the mozparser module changes might need to be handled separately.

Attachment #9061260 - Attachment is obsolete: true
Attachment #9061577 - Attachment is obsolete: true
Attachment #9061578 - Attachment is obsolete: true

(In reply to Ben Campbell from comment #23)

Just some notes on the probe parsers:

Bug 1282098 talks about moving them out of M-C... but I think the consensus was to leave the canonical source in M-C, and have a PyPi package which tracks it (for the data team, which need access to the probe definitions but won't necessarily have M-C).

Yes, that's true. The canonical source lives in m-c, and is mirrored to PyPi for easier access by other teams.

probe-scraper has it's own copy of the probe parsers, with some changes that haven't yet been merged (see https://github.com/mozilla/probe-scraper/issues/6)

Yes, they are slightly diverging for reasons™, but there's a not too concrete plan of getting it to rely on the PyPi package only.

Comment on attachment 9062120 [details] [diff] [review]
allow-extra-probe-definitions.patch

Could I get someone to have a peek at my adding-extra-probe-definitions-into-the-code-generation patch and just give it a bit of a sanity check?
I'm most concerned about being too intrusive in the probe-parsing modules, but I can't really think of another approach to slip extra definitions in there...

(Sorry to pick on you Chris, but I'm not quite sure who best to ask - I figured you can bump it on to someone else if needed :-)
Attachment #9062120 - Flags: feedback?(chutten)
Comment on attachment 9062120 [details] [diff] [review]
allow-extra-probe-definitions.patch

Review of attachment 9062120 [details] [diff] [review]:
-----------------------------------------------------------------

This is looking like a good approach. 

It saddens me that we have such close coupling with the file names of histograms definitions files that you have to edit something inside mozparsers to make it work... but there's really no way out of that without trying to pre-parse Histograms.json, append your probes, and then write it back out again for header generation. And we don't want that.

::: toolkit/components/telemetry/build_scripts/gen_scalar_data.py
@@ +112,5 @@
> +    scalars = []
> +    for filename in filenames:
> +        try:
> +            batch = parse_scalars.load_scalars(filename)
> +            scalars = scalars + batch

I think we should standardize on either `+` or `extend` instead of a mix.

::: toolkit/components/telemetry/build_scripts/mozparsers/parse_histograms.py
@@ +738,5 @@
>  # available, so handle that gracefully.
>  try:
>      import usecounters
>  
> +    FILENAME_PARSERS.append(lambda x: from_UseCounters_conf if x == 'UseCounters.conf' else None)

Huh. This is the first time I've looked this closely at parse_histograms.py. I didn't know we had such tight ties to the file names.

::: toolkit/components/telemetry/moz.build
@@ +202,5 @@
>  event_data = GENERATED_FILES['TelemetryEventData.h']
>  event_data.script = 'build_scripts/gen_event_data.py'
>  event_data.inputs = event_files
>  
> +

Spurious line of whitespace.
Attachment #9062120 - Flags: feedback?(chutten) → feedback+
Attached patch 1427877-telemetry-cc-1.diff (obsolete) — Splinter Review

Here's a test that fails if the Thunderbird probes aren't included in the build. I've also included the two existing Thunderbird probes that are still referenced by the code.

Attachment #9075273 - Flags: feedback?(chutten)
Comment on attachment 9075273 [details] [diff] [review]
1427877-telemetry-cc-1.diff

Review of attachment 9075273 [details] [diff] [review]:
-----------------------------------------------------------------

Looks like `test_telemetry_buildconfig.js` wasn't included in the patch?
Attachment #9075273 - Flags: feedback?(chutten) → feedback-
Attached patch 1427877-telemetry-cc-2.diff (obsolete) — Splinter Review

You're right, adding the file would've been helpful! :-/

Attachment #9075273 - Attachment is obsolete: true
Attachment #9075522 - Flags: feedback?(chutten)
Comment on attachment 9075522 [details] [diff] [review]
1427877-telemetry-cc-2.diff

Review of attachment 9075522 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me!

`<data-steward-hat>` I recommend coming up with a lightweight process for adding new Data Collections via Telemetry in Thunderbird. Doesn't have to be the full [Data Collection Review](https://wiki.mozilla.org/Firefox/Data_Collection), but a quick "This is why we're collecting it, this is who owns it, this is what we're going to do with the resulting data" does wonders for Data Archaeology (when you dig into the past to figure out why we're collecting this, or if it's important that it remain unchanged in a certain way) and for maintaining user trust.`</data-steward-hat>`

::: mail/components/telemetry/tbHistograms.json
@@ +15,5 @@
> +    "kind": "linear",
> +    "high": 100,
> +    "n_buckets": 20,
> +    "description": "Gloda: indexing rate (message/s)",
> +    "alert_emails": [],

I recommend finding a notional email address for someone responsible for these probes, and an expiry version in the not-too-distant future. If it's not owned, maybe it shouldn't be collected.

@@ +20,5 @@
> +    "bug_numbers": [681873]
> +  },
> +  "THUNDERBIRD_TELEMETRY_TEST_CATEGORICAL": {
> +    "record_in_processes": ["main", "content"],
> +    "alert_emails": ["telemetry-client-dev@mozilla.com"],

Might want to swap this.

::: mail/components/test/unit/test_telemetry_buildconfig.js
@@ +49,5 @@
> +  const events = [
> +    ["telemetry.test.thunderbird", "test", "object1"],
> +  ];
> +
> +  // Both test categories should be off by default.

"both"?

@@ +54,5 @@
> +  events.forEach(e => Telemetry.recordEvent(...e));
> +  TelemetryTestUtils.assertEvents([]);
> +  checkEventSummary(events.map(e => (["parent", e, 1])), true);
> +
> +  // Also enable the other test category and see that we record correctly.

"other"?

@@ +64,5 @@
> +
> +// Test Thunderbird histograms are included in the build.
> +add_task(async function test_categorical_histogram() {
> +  let h1 = Telemetry.getHistogramById("THUNDERBIRD_TELEMETRY_TEST_CATEGORICAL");
> +  for (let v of ["CommonLabel", "Label2", "Label3", "Label3", 0, 0, 1]) {

Oh right, I had forgotten that you could use the label ordinal instead of the label itself. That was a design mistake :|
Attachment #9075522 - Flags: feedback?(chutten) → feedback+

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:benc, could you have a look please?
For more information, please visit auto_nag documentation.

Flags: needinfo?(benc)

We know. Quite busy right now.

Assignee: benc → geoff
Flags: needinfo?(benc)
Assignee: geoff → benc

Some things that could be useful to collect:

  • number of accounts + types. server.isGMailServer too
  • size of each mail account (in KB)
  • filelink usage: size sent, times not used even though size was above threshold
  • number of calendars + types
  • startup time
  • system architecture and distributor/packager (for linux) [maybe already available in data]
  • number of enabled add-ons (excluding system ones) [maybe already available in data]
  • address books: number of address books, type, number of contacts
  • chat account type details
  • account setup: success/failure
  • mail volumes: how many mails sent
  • mail volumes: how many mails opened
  • S/MIME:
    • signed: how many opened + how many sent.
    • encrypted: how many opened + how many sent.
    • has identity with encrypt by default set to true
  • filter usage: has filters set up
  • toolbar custimization: has customized toolbar and which ones
  • calendar tasks: using it?
  • has identity set to plain text compositon

Rebased version of the M-C patch to add support for extra probe definitions (basically the same as the one already reviewed in phabricator).

@chutten - if you're still happy with this patch, what are the next steps to get it landed in M-C?

Attachment #9062120 - Attachment is obsolete: true
Attachment #9075274 - Attachment is obsolete: true
Flags: needinfo?(chutten)

This patch adds "thunderbird" as a supported product in the now-mandatory products field in the probe definitions. This seems like the obvious approach, but I'm open to better suggestions that don't fiddle in M-C :- )
I can imagine the products field being quite handy - TB telemetry can ignore all the firefox-specific probes (or add "thunderbird" to any of them we do want to collect).

Attachment #9087634 - Flags: feedback?(chutten)
Attached patch 1427877-telemetry-cc-3.patch (obsolete) — Splinter Review

An updated version of the patch with the new CC probe definition files.
Relies on both the MC patches being applied first.

The new CC telemetry unit tests all seem to pass fine for me locally. But I just realised I've no idea how to run a try build with changesets for both M-C and C-C... I guess we need to get the M-C ones landed first?

Geoff: flagging you up just in case you can think of anything specific I've missed.

Attachment #9075522 - Attachment is obsolete: true
Flags: needinfo?(geoff)
Comment on attachment 9087634 [details] [diff] [review]
1427877-add-thunderbird-to-telemetry-products-1.patch

Review of attachment 9087634 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM
Attachment #9087634 - Flags: feedback?(chutten) → review+
Attachment #9087632 - Flags: review+
Comment on attachment 9087634 [details] [diff] [review]
1427877-add-thunderbird-to-telemetry-products-1.patch

Review of attachment 9087634 [details] [diff] [review]:
-----------------------------------------------------------------

Actually, one thing: I would recommend adding checks to the tests in `toolkit/components/telemetry/tests/python/test_gen_*_data_json.py` and `test_histogramtools_strict.py` to ensure we don't accidentally break the thunderbird product on you.
Comment on attachment 9087635 [details] [diff] [review]
1427877-telemetry-cc-3.patch

Review of attachment 9087635 [details] [diff] [review]:
-----------------------------------------------------------------

f+ from me. I'm not sure on procedure in `mail/**`, but maybe you want someone else to check that these are all done properly for that context.
Attachment #9087635 - Flags: feedback+

(In reply to Ben Campbell from comment #36)

@chutten - if you're still happy with this patch, what are the next steps to get it landed in M-C?

Marking the bug with the checkin-needed keyword will ask the Sheriffs to land the two m-c patches for us. (If we'd done the reviews in Phabricator, I'd task the Lando service to do it).

Do you want to do that?

Flags: needinfo?(chutten)
Keywords: leave-open
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bf126cb4b08f
Add thunderbird to telemetry supported-product flags. r=chutten
https://hg.mozilla.org/integration/mozilla-inbound/rev/6b90caa175cb
Extend telemetry probe registry code-generation to handle extra definition files. r=chutten
Comment on attachment 9087635 [details] [diff] [review]
1427877-telemetry-cc-3.patch

Review of attachment 9087635 [details] [diff] [review]:
-----------------------------------------------------------------

I think the files should be named like the toolkit ones, i.e. lose the "tb" prefix

::: mail/components/telemetry/tbHistograms.json
@@ +32,5 @@
> +      "CommonLabel",
> +      "Label2",
> +      "Label3"
> +    ],
> +    "description": "a testing histogram; not meant to be touched"

Capital A

::: mail/components/telemetry/tbScalars.yaml
@@ +18,5 @@
> +    products:
> +      - 'thunderbird'
> +    kind: uint
> +    notification_emails:
> +      - telemetry-client-dev@mozilla.com

what is the email used for? we should probably set up a telemetry-client-dev@thunderbird.net for this?

::: mail/components/test/unit/test_telemetry_buildconfig.js
@@ +40,5 @@
> +    Telemetry.getSnapshotForScalars("main", aClear)[aProcessName];
> +  return scalars || {};
> +}
> +
> +// Test Thunderbird events are included in the build.

can we move these to jsdoc style /** */

Hmm, looks like landing the M-C parts in this range broke TB big time:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1125aad05d5c39c2453c04ca6d037afaef16c296&tochange=f6a1009171e521c4917dcaccf69aba9093467e99

I'm not really pleased for two reason:

  • apparently this was landed on M-C without any try run.
  • I wasn't CC'ed on the bug, so I came to find this bustage on a Sunday at lunchtime

So what's the way forward here? This should be backed out from M-C until it's ready, right?

Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(benc)

Talked to Jorg on IRC - agreed that backing out the patches best for now.
I suspect it might have been the product-field patch which did it (from the test point of view suddenly the app was "thunderbird" and all the firefox probes have disappeared!). Will check it out and report back tomorrow.

Flags: needinfo?(benc)

(In reply to Jorg K (GMT+2) from comment #46)

Hmm, looks like landing the M-C parts in this range broke TB big time:

Well only the telemetry tests, apparently? That they are broken really doesn't mean a thing yet since Thunderbird doesn't yet do telemetry.

So what's the way forward here? This should be backed out from M-C until it's ready, right?

I think we should hold off and fix the errors. Broken telemetry tests when we know we're not using telemetry is not a problem. Setting MOZ_TELEMETRY_REPORTING=0 could perhaps help in the meantime (I'll have to check)

Status: NEW → ASSIGNED
Type: enhancement → task
Flags: needinfo?(mkmelin+mozilla)

There are an estimated 200 test broken with all X's red on some platforms. This makes sheriffing impossible and it's hard to tell whether something else caused bustage, here an excerpt:
TEST-UNEXPECTED-FAIL | comm/mailnews/db/gloda/test/unit/test_corrupt_database.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/db/gloda/test/unit/test_corrupt_database.js | test_corrupt_databases_get_reported_and_blown_away - [test_corrupt_databases_get_reported_and_blown_away : 108] 3 == 2
TEST-UNEXPECTED-FAIL | comm/mailnews/db/gloda/test/unit/test_index_bad_messages.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/db/gloda/test/unit/test_index_compaction.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/db/gloda/test/unit/test_index_messages_imap_offline.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/db/gloda/test/unit/test_index_messages_imap_online.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/db/gloda/test/unit/test_index_messages_imap_online_to_offline.js | xpcshell return code: 0
TEST-UNEXPECTED-FAIL | comm/mailnews/db/gloda/test/unit/test_migration.js | xpcshell return code: 0

Many failures also in security/manager, toolkit/components/, toolkit/profile/, etc.

For the gloda failures to be fixed, we need the c-c part of this patch to land since those probes now no longer exist.

Sure, but the C-C parts isn't ready to land. So in general, you get all parts ready, you do a try run with all parts, then you land the M-C parts and at the same time notify the C-C sheriff to land the C-C part when M-C merges. Which part of the process did you adhere to here?

https://hg.mozilla.org/mozilla-central/rev/dcd9ee807647b01a5bf493ad0c8e11cfb5c17e8b
Backed out 2 changesets (bug 1427877) for causing Thunderbird xpcshell-bustages. a=backout

Flags: needinfo?(geoff)

Thanks for the try in comment #50, I was going to do one myself. Before the backout I checked some tests locally, including picking toolkit/profile/xpcshell/test_check_backup.js at random.

For further action here, please adhere to that I said in comment #52. Try runs are cheaper than having things backed out from M-C.

And I don't think landing the C-C part as it is will help anyway. It's the firefox tests which are failing, and I think it's because of the second M-C patch (it changes the product from "firefox" to "thunderbird", so suddenly all the firefox probes disappear and the tests fail).
Off the top of my head, I think the options are:

  1. don't treat thunderbird as a separate product from a telemetry POV - ie pretend to be firefox
    pro:
  • less code intrusive
    con:
  • smells bad (it obviously is a different product)
  • TB telemetry collects all the firefox probes, which we probably don't want.
  1. TB is a separate product
    We can add thunderbird as a product for any M-C probes we do want (probes have a list of products they are valid for).
    Still likely to be a bunch of probes which are firefox-only, which need some handling in the M-C unit tests. Maybe we have to early-out whole tests if product is "thunderbird".
    pro:
  • The Right Thing (tm) - separate product, and we can control what data gets collected.
    con:
  • More intrusive. More M-C patching.

Geoff's try is green apart from one which appears(?) to be intermittent (although I've never seen it before):
TEST-UNEXPECTED-FAIL | xpcshell.ini:toolkit/mozapps/extensions/test/xpcshell/test_startup.js | test_modify - [test_modify : 48] "[\"addon2@tests.mozilla.org\"]" == "[]"

BTW, try runs including M-C patches are very easy to do: Push the M-C patches to M-C try without any try syntax, then reference the try changeset in C-C's .gecko_rev.yml. Example in comment #50:
https://hg.mozilla.org/try-comm-central/rev/5addad118939713713ea9efe4592527223c3ccbd

-GECKO_HEAD_REPOSITORY: https://hg.mozilla.org/mozilla-central
-GECKO_HEAD_REF: default
+GECKO_HEAD_REPOSITORY: https://hg.mozilla.org/try
+GECKO_HEAD_REF: 85a003a8633c61b6b1371161eb4cc72e69102d09

which is based on
https://hg.mozilla.org/try/graph/85a003a8633c61b6b1371161eb4cc72e69102d09 (using graph to see the backout).

Documentation here (which doesn't say anything else):
https://wiki.mozilla.org/ReleaseEngineering/ThunderbirdTryServer#Pushing_mozilla-central_patches

I think I've got it all sorted out now. As I suspected, it was adding "thunderbird" as a possible telemetry "product" field which broke loads of M-C tests: a whole heap of the probes were effectively disabled when running as thunderbird, and thus any unit tests relying on them would fail.
I've been through all the failing tests and added "thunderbird" to the probes they rely on, and the following patches will reflect this.

I took the approach of only adding "thunderbird" to the bare minimum of probes - I figured it'd be easier to explicitly choose which extra M-C probes we want to add (and there will be a bunch, especially regarding memory usage and performance). If we just added all of them, it'll be an arse to later on figure out which ones are safe to remove.
One exception was the telemetry-related telemetry probes - I added most of those so we can get feedback and diagnostics on the telemetry itself.

It was interesting to see how the unit tests used the telemetry. Mostly it was checks that particular telemetry probes were working as expected. But there were a few non-telemetry-related tests where various systems used probes to see if things had worked or failed, which I thought was interesting - some systems are more resistant to unit-testing that you'd like, and using the telemetry to gain some insight could be a nice approach, I think.
There were also a couple of places where M-C unit tests were making assumptions that unrelated events would be triggering, which caused problems on TB. Tedious to track down, but easy to fix once you know what's happening.

So. I've got about 4 patches to land (two in M-C and two in C-C), and I've tried to arrange them so they don't require any synchronised landing upon both M-C and C-C at once. Roughly, they'll be:

  1. Small C-C patch to disable the existing TB telemetry probes while we transition
  2. M-C patch to add support for extra definition files (same as what's already been reviewed and approved)
  3. M-C patch to add "thunderbird" as a telemetry product, and to update all the M-C probe definitions and tests accordingly.
  4. C-C patch to restore TB-specific probes and add some C-C unit tests.

This one needs to land first, in C-C.
Just disables the existing TB-specific probes (which are currently in the M-C definitions and will be moved and restored in a subsequent patch).

Oh. No "R?" flags here... Someone care to give it a review for me?

The first M-C patch. Depends on 1427877-cc-disable-tb-specific-probes.patch landing first in C-C.

Just an updated version of the already-reviewed patch to allow pulling extra probe definitions from other files (ie so we can define TB-specific probes).
No real code changes from the previous patch, but there's been a bit of churn, so a rebase seemed prudent.

Attachment #9087632 - Attachment is obsolete: true

(The second M-C patch, to land after 1427877-mc-allow-extra-probe-definitions-3.patch).

  • Defines a new "thunderbird" product enum for telemetry.
  • Adds it to the products field of a bunch of probes required to pass the unit tests.
  • a couple of minor fixes where tests were relied on unrelated Events triggering (in test_LoginManagerParent_onGeneratedPasswordFilledOrEdited.js and test_TelemetryController.js).
Attachment #9087634 - Attachment is obsolete: true

Here's a C-C try job, with the two M-C patches applied (assuming I got my .gecko_rev.yml magic right):
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&selectedJob=264508350&revision=71c6d11ded85ba99d57c8e4b96bcaba19ae42a09

Seems good so far... no more massive telemetry-related test fails this time...

Magnus, can you please review attachment 9089782 [details] [diff] [review].

Flags: needinfo?(mkmelin+mozilla)

At least for the m-c patches, might as well set up phabricator and to the reviews through there.

But why disable the current tb probes? As this will anyway have to land at the same time, why not just land a patch that uses the "new way" at that time?

Flags: needinfo?(mkmelin+mozilla)

(In reply to Magnus Melin [:mkmelin] from comment #66)

At least for the m-c patches, might as well set up phabricator and to the reviews through there.

But why disable the current tb probes? As this will anyway have to land at the same time, why not just land a patch that uses the "new way" at that time?

The idea was to decouple the M-C and C-C changes. The order is still important, but nothing relies on landing stuff in both trees simultaneously.
It sounds like simultaneous landing is doable, but after last time I figured it was best to err on the side of caution.

New plan: I'll leave the existing TB-specific probes in for the M-C patches (and submit them via phabricator). It means another M-C patch at some point to move them over, but we'll likely be submitting the odd patch anyway - to add thunderbird on some of the other M-C probes we'd like to receive.

Attachment #9089782 - Attachment is obsolete: true
Attachment #9089785 - Attachment is obsolete: true
Attachment #9089786 - Attachment is obsolete: true

Sancus: I've got an M-C patch up which adds thunderbird to a bunch of firefox-specific telemetry probes. Some are things we probably want anyway, but a lot are just so all the unit tests pass. We might be able to come up with a better solution for the tests, but I'm inclined to say we should just go with what we've got now, even if it means collecting some superfluous data.
But I figured you should be in the loop on this!
The patch and discussion are at:
https://phabricator.services.mozilla.com/D44426

Flags: needinfo?(sancus)

Chris, would you be open to landing D44425 and D44426? As we discussed on D44426 it'd be nice to add a product-override to avoid adding TB to some of the probes just for test-passing purposes. But for now it'd be nice to just close the loop, get some data flowing and start getting a proper feel for it all.

Flags: needinfo?(chutten)

Here's an update of Geoff's CC patch to add the extra TB-specific probe definitions, and some unit tests to ensure they're being built correctly.

Relies on D44425 and D44426 landing first.

Attachment #9087635 - Attachment is obsolete: true

(In reply to Ben Campbell from comment #71)

Chris, would you be open to landing D44425 and D44426? As we discussed on D44426 it'd be nice to add a product-override to avoid adding TB to some of the probes just for test-passing purposes. But for now it'd be nice to just close the loop, get some data flowing and start getting a proper feel for it all.

File me a follow up bug for the override and I'll set them to land on Monday : ) (getting close to my weekend at this point.)

Flags: needinfo?(chutten)

Thanks Chris! The tyranny of the timezones strikes again, but here's a belated Bug 1579752.

Ah, poop. Looks like we need a rebase. Autoland can't land the patches on its own : |

see also Bug 702561 - Add telemetry reporting for calendar

(In reply to Chris H-C :chutten from comment #75)

Ah, poop. Looks like we need a rebase. Autoland can't land the patches on its own : |

OK, rebased patches submitted.
Handily, moz-phab seems to have identified that I was updating existing patches without any special input from me. Cool! And slightly terrifying - I'm not quite sure what the mechanism for that is...

(and sorry about the delay - was ill all last week!)

Glad you're feeling better.

Pretty sure moz-phab knows because commit messages are amended to include the Phabricator url of the review, and moz-phab chats with phab to make sure the state's okay for uploading. No magic here, as far as I know : )

Pushed by chutten@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fa4c29369074
Extend probe registry code-generation to handle extra definition files r=chutten
https://hg.mozilla.org/integration/autoland/rev/7e72c7602d04
Add thunderbird to telemetry supported-product flags. r=chutten,MattN

So what do I need to land on C-C when this is merged to M-C in three hours?
The changeset from the try run?
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&selectedJob=264508350&revision=71c6d11ded85ba99d57c8e4b96bcaba19ae42a09

Flags: needinfo?(benc)

(In reply to Jorg K (GMT+2) (reduced availability 14-19 of Sept.) from comment #80)

So what do I need to land on C-C when this is merged to M-C in three hours?
The changeset from the try run?
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&selectedJob=264508350&revision=71c6d11ded85ba99d57c8e4b96bcaba19ae42a09

No, the aim was to decouple the changes - the M-C changes can land without borking up C-C.
Attachment 9090921 [details] [diff] is the C-C patch which hooks in some TB-specific telemetry definitions, but I wanted to get the M-C changes in first.

Flags: needinfo?(benc)

And now the M-C patches have landed, here's an update of the basics on the C-C side (a few nit fixes, added some more notes).

magnus: no R? on this bug, hence the Feedback? flag.

Attachment #9090921 - Attachment is obsolete: true
Attachment #9093508 - Flags: feedback?(mkmelin+mozilla)
Comment on attachment 9093508 [details] [diff] [review]
1427877-add-cc-specific-telemetry-support-2.patch

Review of attachment 9093508 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/components/telemetry/Events.yaml
@@ +3,5 @@
> +# file, you can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +# This file contains Thunderbird-specific telemetry Event definitions, which
> +# are added on top of the Firefox ones (in /toolkit/components/telemetry).
> +# To avoid name clashes, all the TB events will be under a "tb" category.

TB -> Thunderbird

::: mail/components/telemetry/README.md
@@ +7,5 @@
> +
> +The probe definitions are used to generate code so the C++ side can use the
> +probe names as enums. This code generation happens out in the M-C side, and
> +there are some build variables which allow us to hook in our extra C-C
> +definitions. See the `MOZ_TELEMETRY_EXTRA_` values in

m-c -> mozilla-central

c-c -> comm-central

Though the language could be changed a bit, the code generation happens... where exactly? "happens build time"?

@@ +21,5 @@
> +For Histograms, we use a `TB_` or `TELEMETRY_TEST_TB_` prefix.
> +
> +(Why not just `TB_`? Because the telemetry test helper functions
> +`getSnapshotForHistograms()`/`getSnapshotForKeyedHistograms()` have an option
> +to filter out histograms with a `TELEMETRY_TEST_` prefix).

Maybe it would be better to change that filtering to just check if the name includes that?

@@ +23,5 @@
> +(Why not just `TB_`? Because the telemetry test helper functions
> +`getSnapshotForHistograms()`/`getSnapshotForKeyedHistograms()` have an option
> +to filter out histograms with a `TELEMETRY_TEST_` prefix).
> +
> +

nit: extra empty line in this file
Attachment #9093508 - Flags: feedback?(mkmelin+mozilla) → feedback+
Flags: needinfo?(sancus)

(In reply to Magnus Melin [:mkmelin] from comment #84)

Though the language could be changed a bit, the code generation happens...
where exactly? "happens build time"?

The README was a bit of an afterthought I've fleshed it out a bit now, so hopefully it should be more generally useful.

@@ +21,5 @@

+For Histograms, we use a TB_ or TELEMETRY_TEST_TB_ prefix.
+
+(Why not just TB_? Because the telemetry test helper functions
+getSnapshotForHistograms()/getSnapshotForKeyedHistograms() have an option
+to filter out histograms with a TELEMETRY_TEST_ prefix).

Maybe it would be better to change that filtering to just check if the name
includes that?

Probably, but I figured it was easiest to avoid an extra M-C patch right now. I only discovered it yesterday and just wanted to make sure it was noted somewhere.

Magnus, if you're OK with this patch can we R+ it?

Try build here:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=1094deed1b554397d8d78b09a0c55da9604d7d2e

Attachment #9093508 - Attachment is obsolete: true
Flags: needinfo?(mkmelin+mozilla)
Attachment #9093773 - Flags: review+
Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(jorgk)
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/e6dd9b872738
Add support for C-C-specific telemetry probes. r=mkmelin

Done here?

Flags: needinfo?(jorgk)
Target Milestone: --- → mozilla71

(In reply to Jorg K (GMT+2) from comment #89)

Done here?

I'm happy to close this one. There are the probes Magnus suggested in Comment 35, but I'd suggest we open individual bugs for each requested probe to better debate and track them.

Yeah let's add more probes and other work in other bugs.

Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.