Closed Bug 910403 Opened 11 years ago Closed 7 years ago

Create endpoint for delivering metrics from MakeAPI (for services like GeckoBoard)

Categories

(Webmaker Graveyard :: MakeAPI, defect)

Product:
Webmaker Graveyard
Component:
MakeAPI
Platform:
x86_64
Windows 7
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED INCOMPLETE

People

(Reporter: beltzner, Assigned: zak.hassan1010, Mentored)

Details

(Whiteboard: webmaker-admin)

Attachments

(1 file, 2 obsolete files)

https://github.com/mozilla/MakeAPI/pull/157
45 bytes, text/x-github-pull-request
cade
: review-
Details | Review 
The objective is to be able to have endpoints that can be called (perhaps with an API key if we're worried about security or privacy) in order to yield the following usage metrics:

 - total number of Makes
 - Total Makes created in the past 24 hours
 - Remixes created in past 24 hours
 - Total Makes created in the past 7 days
 - Remixes created in the past 7 hours

This will require:

 - the creation of the endpoint(s)
 - queries to be written in order to generate the desired metrics, and
 - a JSON format for the metrics

There may be other metrics, but these would represent a good start!
Whiteboard: webmaker-admin
Hi can you assign this bug to me so I can work on it. 

Thanks,
Zak
Hi Zak, thanks for offering to work on this bug!

We love having people build Webmaker with us, but this isn't a good starter bug to dip your toes in. It's a big feature and the looking glass is foggy when we look at how to implement this feature.

I don't know what sort of bugs you find interesting, but if you find one you want to do come chat with us on irc.mozilla.org in #webmaker. We're typically there from 9-5 EST on weekdays. Hope to chat with you soon!
This is a mentored bug being picked up by one of Dave's students. At this point I don't know of it being something that's a pressing need so it's perfectly fine letting him work on it Jon.

Adding Dave/Cade to the CCs.
Assignee: nobody → zak.hassan1010
Status: NEW → ASSIGNED
Whiteboard: webmaker-admin → webmaker-admin mentor=jbuck mentor=cade
To those interested in hacking on this bug, I'm going to describe how I see this bug coming together.

note: I may use some terminology you're not familiar with, if that's the case, ping me on IRC or email me and I'll help you out.

We're going to want to add in a handful of protected routes:

/metrics/makes/all
/metrics/makes/day
/metrics/makes/week
/metrics/remixes/all
/metrics/remixes/day
/metrics/remixes/week

Initially, we can set up a page that displays the metrics to logged in administrators. use the admin console as a model. `/admin/metrics` sounds like a nice place to put it. In the future we could protect the metrics routes with Hawk and allow authenticated calls to it by admin keys.

Code will need to be written so that each route can generate the appropriate query to ElasticSearch, and then process the results into the data we need. You're going to want to familiarize yourselves with Query DSL http://www.elasticsearch.org/guide/reference/query-dsl/ (no easy task, don't try to learn it all, try to find out what will generate the data we need. HINT: look into Facets)

Also important is to define how responses will look in JSON. Consistency is important, and we might want to talk with someone who knows about how data should be formed when passed to various dashboards that we use.

If someone can figure out how to test these, I will be forever in your debt. Actually, scratch that. no merge if no tests are provided :) (which is kind of hypocritical of me, but w/e)
Hi,
I'm working on the implementation and I think I came up with something. Just to make sure I can mimic the same make request's posted to: "/api/make" can someone provide me with 1 example of how the JSON used will look in production:

On the Make API page it displays this:
'{ 
  "maker": "username", 
   make: { ... } 
}';

What data belongs in the make field?
(In reply to Zakeria Hassan from comment #5)
> Hi,
> I'm working on the implementation and I think I came up with something. Just
> to make sure I can mimic the same make request's posted to: "/api/make" can
> someone provide me with 1 example of how the JSON used will look in
> production:
> 
> On the Make API page it displays this:
> '{ 
>   "maker": "username", 
>    make: { ... } 
> }';
> 
> What data belongs in the make field?

To get an idea you can look at the model for a Make (basically, table description) seen here https://github.com/mozilla/MakeAPI/blob/master/lib/models/make.js#L38-L121.
Hi,

All endpoints are functioning correct. If you would like to test this you can use the following endpoints:

  /metrics/makes/all
  /metrics/makes/day
  /metrics/makes/week
  /metrics/remixes/all
  /metrics/remixes/day
  /metrics/remixes/week

>Initially, we can set up a page that displays the metrics to logged in administrators. use the admin console as a model. `/admin/metrics` sounds like a nice place to put it. In the future we could protect the metrics routes with Hawk and allow authenticated calls to it by admin keys.

Note: I'm finished with the backend code. I would like to make a pull request and submit my code to Mozilla. 

I'd like to ask Cade a few questions about the UI as well has admin keys. Perhaps we could create new bug for the UI for the metrics & Hawk Authentication.
@Zak: I've left some comments in-line with your pull request on GitHub. Feel free to email me or find me on IRC if you have any questions.
@Cade: I've addressed all comments and I've pushed my latest code onto GitHub. Now I'm just waiting for my pull request to get approved.

Thanks,
Zak
Can you add the link to the pull request as an attachment to the bug?
This is the latest pull request and it highlights all the changes I've made. I've run the 'npm tests' and everything passed. Also I fixed every styling comment you mentioned in the past pull request & all code problems. Thank you again. Sorry for having go back and forth with reviewing this bug.
Attachment #814708 - Flags: feedback+
Just cleaning up so that we have the pull request link vs. compare branch.

We talked in class, and he's still going to be doing a rebase, once he gets an r+.
Attachment #814708 - Attachment is obsolete: true
Attachment #815369 - Flags: review?(cade)
@cade: I am done all required fix's for this patch. Including a UI to demonstrate that it works. I used native javascript so it should work cross platform even on Firefox/Safari/IE. Anyway let me know how I did. I'm excited to get my first bug push to Mozilla.

Thanks,
Attachment mime type: text/plain → text/x-github-pull-request
Comment on attachment 815369 [details] [review]
https://github.com/mozilla/MakeAPI/pull/153

I left comments in the PR for Zak about a month ago - have not heard back from him since.
Attachment #815369 - Flags: review?(cade) → review-
Thank you for the review. I'm sorry I wasn't quick to respond. I will fix it now.
Hi Cade,

Now after looking through the bug. I realized what happened. I commented on 2013-10-10 which was 2 months ago and I did not get a reply until a month ago. And I fixed every issue except the UI portion. I think I could quickly fix the UI and rebase. Before I go forward can you let me know if this is the only fix left because I got a few reviews for this bug specifically and there seems to be more to fix after I fix what is requested. Just for my understanding is the UI portion all that is left and why isn't it possible for us to use the data table because you did mention in a previous comment the following:

> use the admin console as a model. `/admin/metrics` sounds like a nice place to put it.

Questions:
1) Is the code that queries ElasticSearch correct?
2) Is there anything else left to resolve this bug?
I am ready and willing to fix this bug. I will fix the UI issue and I will update the pull request. Sorry for the delay. Thank you for your patience. Please disregard my previous comment.
Zak, what's the status of this bug?
I fixed the backend code for this bug long time ago. The issue that remains is how I display metrics in the UI. Cade requested that I not use the datatable that is used in the /admin page. Since this was a patch from 3 months ago code has changed and as a result merge conflicts are occuring for me. I going to close the pull request and try to resolve this merge conflict. Once I have everything working then I will submit a new one. 

Thank you,
Zak
Comment on attachment 815369 [details] [review]
https://github.com/mozilla/MakeAPI/pull/153

Okay code has been pushed all the code in with. This bug is complete.
Attachment #815369 - Flags: review- → review?(cade)
I will take a look at this asap, thanks Zak!
Zak: Create a new attachment, and mark the old attachment as obsolete by selecting the old attachment in the "obsoletes" section of the new attachment creation form.
Attachment #815369 - Attachment is obsolete: true
Attachment #815369 - Flags: review?(cade)
Comment on attachment 8349488 [details] [review]
https://github.com/mozilla/MakeAPI/pull/157

This got a review a while ago, I forgot to update this attachment. See github for review comments
Attachment #8349488 - Flags: review?(cade) → review-
Okay I'll work on this immediately. Thank you for the review.
Mentor: jbuck, cade
Whiteboard: webmaker-admin mentor=jbuck mentor=cade → webmaker-admin
The makeapi is no longer in operation, and isn't developed or maintained.

closing out this issue.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: