Closed Bug 659794 Opened 13 years ago Closed 13 years ago

Expose performance information in the API

Categories

(addons.mozilla.org Graveyard :: API, defect, P1)

x86
BSDI
defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: peregrino, Assigned: kumar)

References

Details

As a first step at providing information about addons performance in the Addon Manager, a way to get that data from AMO is needed.

Right now the API doesn't provide this data. Maybe adding a new section inside the addon item called "performance" where all the performance metrics will be sent? Right now IIRC only the ts_slowness is measured, but in the future hopefully more metrics will be added.
Hernan is working on surfacing this in the Add-ons Manager during his internship and it shouldn't be too difficult to do.

We should expose the percentage and OS data we have, but we'll also probably want a flag that says whether it's over the threshold or not too.
Target Milestone: --- → Q2 2011
(In reply to comment #1)
> Hernan is working on surfacing this in the Add-ons Manager during his
> internship and it shouldn't be too difficult to do.
> 
> We should expose the percentage and OS data we have, but we'll also probably
> want a flag that says whether it's over the threshold or not too.

This sounds great.  For now, we should just treat this visually the same way we treat incompatible messaging - yellow not red, scary but not deadly.
Blocks: 597282
Hardware: x86 → All
This would be a possible way to send this data in the XML:

<addon id="1843">
    <name>Firebug</name>
    <type id="1">Extension</type>
    <guid>firebug@software.joehewitt.com</guid>
    <version>1.7.1</version>
    <status id="4">Fully Reviewed</status>
    <authors>
        <author id="9265">
            <name>Joe Hewitt</name>
            <link>https://addons.mozilla.org/en-US/firefox/user/9265/?src=api</link>
        </author>
    </authors>
    <summary>Firebug integrates with ...</summary>
    <description>Firebug integrates with ...</description>
    <icon>https://static-cdn.addons.mozilla.net/en-US/firefox/images/addon_icon/1843-32.png?modified=1305827346</icon>
    <compatible_applications>
        <application>
            <name>Firefox</name>
            <application_id>1</application_id>
            <min_version>3.6</min_version>
            <max_version>5.*</max_version>
            <appID>{ec8030f7-c20a-464f-9b0e-13a3a9e97384}</appID>
        </application>
    </compatible_applications>
    <all_compatible_os>
        <os>ALL</os>
    </all_compatible_os>
    <rating>5</rating>
    <performance>
      <startup_time>74</startup_time>
    </performance>
</addon>

Pay attention to the <performance> element, that includes the startup_time measurement. Other metrics could be added later as children of performance.
Hardware: All → x86
Target Milestone: Q2 2011 → ---
<startup_time> is measured per-platform and per-browser version.  Giving you all those numbers doesn't help you though without the baseline measurements for how long a browser takes to start without any add-ons.  And the third piece of information would be the threshold the site shows warnings at (or, at least an attribute on the startup_time element for whether it was above the threshold or not).
Well we only care about the additional startup time that this add-on has added for the platform we are requesting the API results for don't we?
(In reply to comment #5)
> Well we only care about the additional startup time that this add-on has
> added for the platform we are requesting the API results for don't we?

An add-on API URL is: https://services.addons.mozilla.org/en-US/firefox/api/1.5/addon/1843

That doesn't include the platform in it.  For the example data in comment 3, Firebug, we have data for Fx 3.6.15, Fx 4.0, Fx 4.0.1, and OS X 10.5.8, Fedora 12, Win 5.1, Win 6.1, and all the combinations of the above.
Firefox requests https://services.addons.mozilla.org/%LOCALE%/firefox/api/%API_VERSION%/search/%TERMS%/all/%MAX_RESULTS%/%OS%/%VERSION%?src=firefox

though that still isn't enough to track the OS version differences I guess.
Is the AMO site going to show different information depending on what version of what OS you are viewing from?
It doesn't right now.  We generally opt to show you more, not less, via the API.  For example, the several <install> elements with different OS's as attributes.
(In reply to comment #7)
> Firefox requests
> https://services.addons.mozilla.org/%LOCALE%/firefox/api/%API_VERSION%/
> search/%TERMS%/all/%MAX_RESULTS%/%OS%/%VERSION%?src=firefox
> 
> though that still isn't enough to track the OS version differences I guess.

And what about adding the %OS_VERSION% to that URL? Then the performance data could come with the data for that specific version of the OS.

Other solution could be to get the data for all the OS versions that have results for the target browser version that is being requested. Like if you request info about an addon for FF6 running on windows, you get all the metrics for the different windows versions.
Then in the AOM we only show the info for the actual OS version.

The last solution could be to just get all the data that's available, but that seems overkill to me, much more info than what is really needed.
I can offer you something like this:

<performance>
  <application name="fx" version="3.6.15">
    <platform name="MacOSX" version="10.5.8">
      <result type="ts" baseline="500" average="830" />
    </platform>
    <platform name="WINNT" version="6.1">
      <result type="ts" baseline="500" average="830" />
    </platform>

    ...

  </application>

  ...

</performance>


If you put %OS% and %OS_VERSION% in the URL we can restrict it to just those two.  Would you rather see no results or all results if those are left out of the URL?
We should make sure that whatever info AMO shows online matches what we show in Firefox. What are the plans for showing info when the users is running a different OS version (or even OS) to that that the tests were run on? Likewise for Firefox version.
We have a base "slowness" number that is an average of all the other numbers put together[1].  That's what we're showing users right now on that /performance page.  I don't think it's a great system.

I don't know of future plans to split it out by OS/platform, but I imagine we'll do that at some point.

[1] https://github.com/jbalogh/zamboni/blob/master/apps/perf/tasks.py#L14
Any news on what we should expose? 
I'm not entirely comfortable showing an avg of all the platforms on the addon manager... Those values vary a lot between OSes and in the addon manager the info is supposed to be of the things you have installed, I think. So I would like to get the values at least broken down by OS.
OS: All → BSDI
What do you guys want to do here?
Target Milestone: --- → Q3 2011
(In reply to comment #15)
> What do you guys want to do here?

We discussed this bug yesterday in the Add-ons Manager meeting and should have updated notes here soon for what we'd need from the API.
In the meeting we set a couple of things:

* Not be specific about performance numbers
    Only show a general message saying Firefox is slowed down by this addon
    but not showing the exact slowdown factor. 

* Warning only shown if we get a warning from the API
    If a result from the tests is "bad" and hurting performance, a flag
    should appear on the result indicating it.

* Only check for OS, not Firefox version
    We will only care about the OS name and version, following this logic:

      If name and version match → use it
        else if name match → use it (use the nearest version we can get)
          else → don't show anything

So what we need from the API here is something like Wil said in Comment #11, but always returning all the performance data we have (not restricting by OS). Then if some result is hurting the performance (according to some criteria in AMO side), that result should be flagged with `aboveThreshold="true"` or a better suitable flag name.

What this leads me to is that we don't even need the performance numbers at all. We could only get a list of OS names and versions on which this version of firefox is having performance issues with the addon. 
But in the other hand, if we ever get a better performance reporting system and we would like to show the exact numbers, the API will have to change again...
In an API I'd like to give as much data as we can and let other people decide if they want to show numbers or not.  How about this:

<performance>
  <application name="fx" version="3.6.15">
    <platform name="MacOSX" version="10.5.8">
      <result type="ts" baseline="500" average="830" above_threshold="true" />
    </platform>
    <platform name="WINNT" version="6.1">
      <result type="ts" baseline="500" average="530" />
    </platform>

    ...

  </application>

  ...

</performance>

We don't currently have a garbage collection on these perf numbers and I suppose we'll want to add one at some point or this page will get enormous.

Assigning to Kumar in the hopes that this is what you want and we can get this in next week.  Please give the thumbs up soon if you're happy with it.
Assignee: nobody → kumar.mcmillan
Target Milestone: Q3 2011 → 6.1.6
(In reply to comment #18)
> <performance>
>   <application name="fx" version="3.6.15">
>     <platform name="MacOSX" version="10.5.8">

Can we please keep the platform specifiers as we are already using for pings and the discovery pane? Introducing new names is way confusing especially when it happens in the same module. So far for OS X this should be Darwin.
(In reply to comment #19)
> (In reply to comment #18)
> > <performance>
> >   <application name="fx" version="3.6.15">
> >     <platform name="MacOSX" version="10.5.8">
> 
> Can we please keep the platform specifiers as we are already using for pings
> and the discovery pane? Introducing new names is way confusing especially
> when it happens in the same module. So far for OS X this should be Darwin.

We're using whatever Alice's team is giving us.  I don't know what their list is.
(In reply to comment #20)
> (In reply to comment #19)
> > (In reply to comment #18)
> > > <performance>
> > >   <application name="fx" version="3.6.15">
> > >     <platform name="MacOSX" version="10.5.8">
> > 
> > Can we please keep the platform specifiers as we are already using for pings
> > and the discovery pane? Introducing new names is way confusing especially
> > when it happens in the same module. So far for OS X this should be Darwin.
> 
> We're using whatever Alice's team is giving us.  I don't know what their
> list is.

Internally I think all we have are the identifiers we use in the discovery pane so somewhere we are going to have to translate the test OSs to those. This could be on Alice's side, AMO's side or Firefox's side. My preference would be Alice's so we don't have to keep a hardcoded mapping anywhere, but doesn't AMO already have that mapping anyway for the OS in the install elements in the API results?
Other than that I think the proposed spec looks great though I'd like to see an appID attribute on application for the App's GUID.
AMO maintains a separate set of apps from Alice's just because we didn't know what she would send us and didn't want to deal with the complexity.  I agree this should be changed on her end.
I'm using the names that are available in Firefox (OS_TARGET), so matching OS names in the API would be fantastic.
Severity: enhancement → normal
Priority: -- → P1
Target Milestone: 6.1.6 → 6.1.7
CCing Alice about the naming stuff.
Is this the format that you are wanting to change?

https://wiki.mozilla.org/Buildbot/Talos/DataFormat#AMO
(In reply to comment #26)
> Is this the format that you are wanting to change?
> 
> https://wiki.mozilla.org/Buildbot/Talos/DataFormat#AMO

The question is about the OS name that we will have to check from the Addon Manager side. We're using this names: https://developer.mozilla.org/en/OS_TARGET, so matching ones would be desirable.
Won't that throw leopard/snow leopard or winxp/win7/win764 into the same bucked, os wise?
(In reply to comment #28)
> Won't that throw leopard/snow leopard or winxp/win7/win764 into the same
> bucked, os wise?

They share the name, but have different versions IIRC.
Target Milestone: 6.1.7 → 6.1.8
This has been implemented in https://github.com/jbalogh/zamboni/commit/3be146d

You can check out Firebug on the staging server:
https://addons.allizom.org/en-US/firefox/api/1.5/addon/1843

It matches up here but let me know if any numbers seem off elsewhere.
https://addons.allizom.org/en-US/firefox/performance/

Some things to note:
- The API version has not changed
- See Comment #18 for XML format
- Only the 20 most recent performance results are included
- the apps/versions/OS data (names, etc) has not changed. Let me know if anything needs updating
- above_threshold is true when the addon starts up 25% (or more) slower than the baseline. This threshold is configurable
- the above_threshold attribute might be missing from a <result/> element. It only applies to startup time.

From a quick siege benchmark, the new code for this does not seem to adversely affect performance. We'll keep an eye on the stats though once it's pushed out: 
https://graphite-sjc.mozilla.org/render/?width=975&height=389&_salt=1311792249.74&target=stats.timers.amo.view.api.views.render_xml.GET.lower&target=stats.timers.amo.view.api.views.render_xml.GET.upper_90&target=stats.timers.amo.view.api.views.render_xml.GET.mean

This will get deployed tomorrow but we will switch it off if we run into performance issues (or if QA wants to switch it off :)).
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Great news! sadly I'll be connected only through mobile devices until Friday, so I won't be able to check this soon.
I have quickly checked the given XML snippet for Firebug. As seen we don't use the available platform names as given by comment 27. The check-in comment only mentions comment 18 with the first proposal. Can we please get an update on that what is really wanted and needed? Reopening the bug until this question has been solved.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
We'll want to stick with the data format as described in:

https://wiki.mozilla.org/Buildbot/Talos/DataFormat#AMO

The talos/graph server side has not be altered and still meets the written standard.
If the Talos side needs to be altered please file a new bug under the Talos/graph server product.  AMO just passes through whatever Talos gives us.
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Can we do this conversion in AMO? It can't be altered on Talos' side and it seems better to have it server-side than client-side in case there are changes later on.
That would be putting a maintenance burden in the middle of the pipeline.  Why do you say this can't be altered on Talos' side?
After deploying this we ran into a data exception so we disabled it. There is a fix in place but we're going to do some deeper analysis before re-enabling it. After bug 675077 I'll have a new production DB snapshot to work with on PAMO.
Status: RESOLVED → REOPENED
Depends on: 675077
Resolution: FIXED → ---
Target Milestone: 6.1.8 → 6.1.9
Target Milestone: 6.1.9 → Q3 2011
Target Milestone: Q3 2011 → 6.2.1
Depends on: 680953
Target Milestone: 6.2.1 → 6.2.2
This was turned on today
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Should I see this on today's nightly Addons Manager?
You can see this if you look at the XML coming off the AMO API.  I don't know when it will be exposed in the client.
(In reply to Wil Clouser [:clouserw] from comment #41)
> You can see this if you look at the XML coming off the AMO API.  I don't
> know when it will be exposed in the client.

Many thanks for answering me so quickly, alas don't know enough to understand what you mean. If you care to explain me some further, I'd really appreciate it! Should you think this isn't the correct place to do that, no problem at all.
(In reply to Henrik Skupin (:whimboo) from comment #43)
> Marking as verified fixed by a query like:
> 
> https://services.addons.mozilla.org/en-US/firefox/api/1.5/search/adblock/all/
> 30/Darwin/6.0a2?src=firefox

Henrik, was your comment for me? If so, many thanks! I could see the performance there, but how did you obtain the link above?
Could you please tell me what does  the following mean?

ts" baseline="536.21" average="611.21" above_threshold="false

I would really appreciate it!
(In reply to Gabriela from comment #44)
> (In reply to Henrik Skupin (:whimboo) from comment #43)
> > Marking as verified fixed by a query like:
> > 
> > https://services.addons.mozilla.org/en-US/firefox/api/1.5/search/adblock/all/
> > 30/Darwin/6.0a2?src=firefox
> 
> Henrik, was your comment for me? If so, many thanks! I could see the
> performance there, but how did you obtain the link above?
> Could you please tell me what does  the following mean?
> 
> ts" baseline="536.21" average="611.21" above_threshold="false
> 
> I would really appreciate it!

Gabriela, that was an informative link, so that others can verify is working. This wont be exposed directly to the user, it will be parsed and shown as a warning in the addons manager. If you like to keep track of that, look at bug #597282
(In reply to Hernán Rodriguez Colmeiro (:peregrino) from comment #45)
> (In reply to Gabriela from comment #44)
> > (In reply to Henrik Skupin (:whimboo) from comment #43)
> > > Marking as verified fixed by a query like:
> > > 
> > > https://services.addons.mozilla.org/en-US/firefox/api/1.5/search/adblock/all/
> > > 30/Darwin/6.0a2?src=firefox
> > 
> > Henrik, was your comment for me? If so, many thanks! I could see the
> > performance there, but how did you obtain the link above?
> > Could you please tell me what does  the following mean?
> > 
> > ts" baseline="536.21" average="611.21" above_threshold="false
> > 
> > I would really appreciate it!
> 
> Gabriela, that was an informative link, so that others can verify is
> working. This wont be exposed directly to the user, it will be parsed and
> shown as a warning in the addons manager. If you like to keep track of that,
> look at bug #597282

Hernán, many thanks! I'm already keeping track of that bug. I thought this one was (per description: As a first step at providing information about addons performance in the Addon Manager, a way to get that data from AMO is needed) about having data performance in AMO.
(In reply to Gabriela from comment #46)
> Hernán, many thanks! I'm already keeping track of that bug. I thought this
> one was (per description: As a first step at providing information about
> addons performance in the Addon Manager, a way to get that data from AMO is
> needed) about having data performance in AMO.

Getting data *from* AMO, not *in* AMO :) 

Anyway, localization will be needed but not here, this was mainly for getting the numbers publicly available. Thanks for the interest!
(In reply to Hernán Rodriguez Colmeiro (:peregrino) from comment #47)
> (In reply to Gabriela from comment #46)
> > Hernán, many thanks! I'm already keeping track of that bug. I thought this
> > one was (per description: As a first step at providing information about
> > addons performance in the Addon Manager, a way to get that data from AMO is
> > needed) about having data performance in AMO.
> 
> Getting data *from* AMO, not *in* AMO :) 
> 
> Anyway, localization will be needed but not here, this was mainly for
> getting the numbers publicly available. Thanks for the interest!

No problem, it's good to know this bug is fixed! I hope bug #597282 will be fixed soon too!
(In reply to Henrik Skupin (:whimboo) from comment #43)
> Marking as verified fixed by a query like:
> 
> https://services.addons.mozilla.org/en-US/firefox/api/1.5/search/adblock/all/
> 30/Darwin/6.0a2?src=firefox

We still have the names as they come out of Talos, and I'm reluctant to have this mapped in the client side as comment #35 says.
Can you file a separate bug specifying what you'd like changed?  Thanks
Depends on: 693209
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.