Closed
Bug 1211164
Opened 9 years ago
Closed 9 years ago
Collect JS deprecated language extension telemetry for Add-ons.
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla44
Tracking | Status | |
---|---|---|
firefox44 | --- | fixed |
People
(Reporter: arai, Assigned: arai)
References
Details
Attachments
(1 file)
5.33 KB,
patch
|
till
:
review+
benjamin
:
feedback+
|
Details | Diff | Splinter Review |
Derived from bug 1208835.
Currently we're collecting telemetry for deprecated language extension only for web content (bug 1133900).
If we're going to remove some deprecated feature, we'll also need to know how much it's used in Add-ons, to reduce addon-compat issue after removing it. Of course it should be corrected separately from web content.
Assignee | ||
Comment 1•9 years ago
|
||
English is difficult :P
Summary: Correct JS deprecated language extension telemetry for Add-ons. → Collect JS deprecated language extension telemetry for Add-ons.
Assignee | ||
Comment 2•9 years ago
|
||
If we're going to collect all values, it could be done in JSCompartment::addTelemetry and JSCompartment::reportTelemetry.
Assignee: nobody → arai.unmht
Attachment #8669356 -
Flags: review?(till)
Comment 3•9 years ago
|
||
Comment on attachment 8669356 [details] [diff] [review]
Collect JS deprecated language extension telemetry for Add-ons.
Review of attachment 8669356 [details] [diff] [review]:
-----------------------------------------------------------------
Nice, thank you. r=me with nit addressed. However, I asked for a telemetry peer's review, too, so please wait for that before landing.
@bsmedberg, we have exactly this probe for web content already. We now want to also introduce it for addons, so we know when it is safe to remove deprecated features not only from the web but also from chrome JS. (We explicitly didn't include addons or chrome code for the first probe because we had lots of usages through the addons SDK and didn't want web usage to be swamped out. Now things have changed somewhat in the SDK and we want to learn about addons, too.)
::: js/src/jscompartment.cpp
@@ +1094,5 @@
> void
> JSCompartment::addTelemetry(const char* filename, DeprecatedLanguageExtension e)
> {
> + // Only report telemetry for web content and add-ons, not chrome JS.
> + if (!addonId && (isSystem_ || !filename || strncmp(filename, "http", 4) != 0))
In practice it might not make a difference, but this check does something slightly different than the above: the way it is now, if there is an addonId it doesn't make a difference if isSystem_ is set or not. Before, and in JSCompartment::reportTelemetry, if isSystem_ is set we always bail. Perhaps change it to
if (isSystem_ || (!addonId && (!filename || strncmp(filename, "http", 4) != 0)))
Attachment #8669356 -
Flags: review?(till)
Attachment #8669356 -
Flags: review?(benjamin)
Attachment #8669356 -
Flags: review+
Comment 4•9 years ago
|
||
Comment on attachment 8669356 [details] [diff] [review]
Collect JS deprecated language extension telemetry for Add-ons.
Do you also want to record the addon ID in a keyed histogram, or is counting them across all addons sufficient?
I don't think this should be expires_in_version: never unless you have a product manager or QA lead who is committed to monitoring this permanently. Do you?
Flags: needinfo?(arai.unmht)
Assignee | ||
Comment 5•9 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #4)
> Comment on attachment 8669356 [details] [diff] [review]
> Collect JS deprecated language extension telemetry for Add-ons.
>
> Do you also want to record the addon ID in a keyed histogram, or is counting
> them across all addons sufficient?
Counting across all addons will be sufficient, as this is the complementary for JS_DEPRECATED_LANGUAGE_EXTENSIONS_IN_CONTENT, that we're also monitoring only the total number. Telemetry probes for add-ons are once added by bug 1054630 as part of JS_DEPRECATED_LANGUAGE_EXTENSIONS_IN_CONTENT, and excluded from it by bug 1133900.
I'm not going to contact each add-on's developer for now. Just to figure out how much those deprecated features are used in add-ons, and when we can remove it safely.
I think we should collect data per each add-ons only when the number stop decreasing at high point.
> I don't think this should be expires_in_version: never unless you have a
> product manager or QA lead who is committed to monitoring this permanently.
> Do you?
I think this should be treated in same way as JS_DEPRECATED_LANGUAGE_EXTENSIONS_IN_CONTENT.
so... who is the product manager currently working on it?
Flags: needinfo?(arai.unmht)
Comment 6•9 years ago
|
||
In case you're asking me that last question: I don't know. Naveed, do you have embedded QA on the JS team to help with this kind of thing?
Flags: needinfo?(nihsanullah)
Comment 7•9 years ago
|
||
Not sure what QA means in this context, but personally I check this Telemetry probe regularly to see which features we can potentially remove from the engine...
Comment 8•9 years ago
|
||
Comment on attachment 8669356 [details] [diff] [review]
Collect JS deprecated language extension telemetry for Add-ons.
If Jan is signed up to monitor these, please add "alert_emails": ["jdemooij@mozilla.com"] into Histograms.json for these metrics so that we know who owns monitoring them.
data-collection-review+ with that change
Attachment #8669356 -
Flags: review?(benjamin) → feedback+
Assignee | ||
Comment 9•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/01cdd3e5bd7153dfdc47d865538e15e4754dc0cf
Bug 1211164 - Collect JS deprecated language extension telemetry for Add-ons. r=till,bsmedberg
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Updated•9 years ago
|
Assignee: arai.unmht → emanuel.hoogeveen
Flags: needinfo?(nihsanullah)
You need to log in
before you can comment on or make changes to this bug.
Description
•