Add Librato integration #648

RafaelloLollipop · 2016-04-11T13:17:07Z

Add Librato integration

smarx · 2016-04-11T13:17:08Z

Automated message from Dropbox CLA bot

@VallHer, it looks like you've already signed the Dropbox CLA. Thanks!

timabbott · 2016-04-11T17:21:05Z

templates/zerver/integrations.html

+    name <code>librato</code>.</p>
+
+    <p>Next, on your <a href="/#settings" target="_blank">Zulip settings
+    page</a>, create an Librato bot. Please note the bot name and API key. Then:</p>


s/an/a/ before Librato bot

timabbott · 2016-04-11T17:43:58Z

@VallHer thanks for working on this! I posted a few comments.

timabbott · 2016-04-11T17:44:14Z

Also, you should squash the two commits together.

timabbott · 2016-04-11T17:47:27Z

It'd be great if we could get feedback from the Librato folks or someone who has used Librato on the message format -- I pinged the person who opened the original issue to see if he's willing to take a look. The format is relatively verbose compared to how most of our integrations read, but I'm not sure if that's the nature of the Librato service.

RafaelloLollipop · 2016-04-11T19:50:09Z

@timabbott
Slack example:
http://wrzuc.se/images/570bfee268230.png

I can change it to slack style, should I?

timabbott · 2016-04-11T19:53:36Z

I think that's mostly reasonable, except that Slack doesn't have topics/threading, and we should move some part of that content into the topic (is "runbook" the machine here? If so maybe "runbook ToHighTemperatureAlert" would be a good thread name?).

It might be helpful to have a few more example Librato alerts in the fixtures that we can work with...

zack-ahdach · 2016-04-12T08:41:22Z

@timabbott I've reviewed screenshot 006 and as far as I'm concerned it looks fine. The only improvement I can think of is adding some color to easily recognise wether an alert is new or resolved.
For reference, here's a "new" alert (green) and a "resolved" alert (blue) from Librato in Slack.

timabbott · 2016-04-12T20:46:08Z

Cool. We don't currently have a way to do that variable color thing in Zulip, but it might be a formatting capability worth adding; shouldn't be too hard to do. Certainly I don't think green for new and blue for resolved is the right approach :). @VallHer does this give you the info you need to do a revised version?

Also I guess this needs to be rebased across the tests refactoring I merged yesterday.

RafaelloLollipop · 2016-04-12T22:57:19Z

@zack-ahdach
Thanks for feedback

@timabbott

update structure of notification (006.png)
add new fixture with longer example
add handling of "cleared" message (+tests)
default topic is unique for error name
add option to change default topic

EDIT:
I find that i can implement charts functionality, so don't close this request yet!

timabbott · 2016-04-13T21:13:49Z

zerver/views/webhooks/librato.py

+
+@api_key_only_webhook_view
+@has_request_variables
+def api_librato_webhook(request, user_profile, stream=REQ(default='librato'), topic=REQ(default=None)):


This line is too long; the topic argument should be on a second line.

timabbott · 2016-04-13T21:15:12Z

OK will wait for the charts stuff to show up!

RafaelloLollipop · 2016-04-13T21:56:45Z

Ok I added option to handle charts. We need to wait for guys from librato for better solution.

RafaelloLollipop · 2016-04-18T19:54:30Z

Ok i found how to write service for Librato, however i need also some more time beacuse it is in Ruby and i don't have experience at all.

timabbott · 2016-04-19T20:08:19Z

What do you mean by "write service for Librato"? Is the idea to do a non-webhook integration?

RafaelloLollipop · 2016-04-19T20:25:05Z

Librato webhooks allow us to handle only 'Alert' module.

To use snaps, we need to create our integration(service). It looks the same as webhook at all.

Easy way: we can totally copy Slack service and it will be (propably) work.
However i think it is good possibility to little expand menu and add for example topic input.

timabbott · 2016-04-22T01:04:50Z

Ahh, OK, that makes sense!

timabbott · 2016-04-22T01:09:13Z

templates/zerver/integrations.html

+
+      <p><b>Congratulations! You're done!</b><br /> When a snapshot comes, you'll get a Zulip notification that looks like this:</p>
+
+      <p><img class="screenshot" src="/static/images/integrations/librato/009.png" /></p>


Can you retake these screenshots showing the stream/topic recipient bar? That's how the screenshots work in our other integrations. While you're doing it, can you give the bot the name "Librato Bot" to match the bot naming style of the other integrations.

Also, you should line wrap the text in a lot of these paragraphs, like we do for the other integrations.

Thanks!

timabbott · 2016-04-22T01:09:36Z

Posted a few comments on the details of the documentation for this.

Thanks for building this @VallHer, it's looking really good.

timabbott · 2016-05-10T22:27:58Z

@VallHer how are you doing on this? If you're too busy to finish this, let me know and perhaps someone else can pick it up.

RafaelloLollipop · 2016-06-07T17:51:43Z

@timabbott
I have a problem with ruby service. To make this integration completely we need to copy almost slack integration and provide zulip one. However i don't have any experience in Ruby.
https://github.com/librato/librato-services

Or i just can finish integration without snapshot. However it will be better if someone can help me here.

timabbott · 2016-06-07T17:54:52Z

@VallHer if we just finished and merged the Librato integration without snapshot support, would the work done need to be thrown away to add snapshot support, or would it be a more incremental extension? If the extension would be incremental, I'd recommend we finish and merging without snapshot support, and adding snapshot support can be a future project.

Otherwise, @TomaszKolek do you know Ruby / are you interested in learning it to help with this?

TomaszKolek · 2016-06-07T18:18:20Z

@timabbott
Some time (~3 years) ago I wrote one project in RoR. But yes, I'm interesting in learning it!

TomaszKolek · 2016-07-13T17:55:23Z

@timabbott
I did a research and my conclusions are:

We should end this PR and we merge it as a webhook integration.
We should create service (probably with cooperation with somebody from Librato)
I guess we'll be waiting some time before merging it into Librato so it's not worth to wait for it with a merge.

@VallHer please confirm my conclusions. Or not If I'm talking nonsense.
Also please tell me if you'd like to end this ticket (I mean adjusting with new style webhook/tests) or I can do it?

RafaelloLollipop · 2016-07-13T21:56:32Z

@TomaszKolek
Yes, you are right.

I will be glad if you can do it instead of me. I actually lost 'connection' with project and forgot basic things.

timabbott · 2016-07-14T04:25:21Z

@TomaszKolek that's reasonable to me; I think we should go ahead and merge a webhook integration. Then if we find time to add a Librato service, we can have that service use a similar format to the existing webhook integration for sending data over, to reuse the existing code.

timabbott · 2016-09-14T04:40:43Z

Closing this since development is proceeding on #1749.

RafaelloLollipop changed the title ~~Librato integration~~ Add Librato integration Apr 11, 2016

timabbott reviewed Apr 11, 2016
View reviewed changes

timabbott mentioned this pull request Apr 11, 2016

Add Librato.com as an integration #68

Closed

RafaelloLollipop force-pushed the librato-integration branch 2 times, most recently from 8ed3d2c to e1a0aee Compare April 11, 2016 19:19

RafaelloLollipop force-pushed the librato-integration branch 4 times, most recently from f52a8ec to 5c0807e Compare April 12, 2016 22:39

RafaelloLollipop force-pushed the librato-integration branch 2 times, most recently from 71d6f15 to 8f296cd Compare April 13, 2016 12:00

timabbott reviewed Apr 13, 2016
View reviewed changes

RafaelloLollipop force-pushed the librato-integration branch from 8f296cd to aed8095 Compare April 13, 2016 21:51

RafaelloLollipop force-pushed the librato-integration branch 2 times, most recently from f388baa to 7a0732e Compare April 14, 2016 11:43

RafaelloLollipop force-pushed the librato-integration branch from 7a0732e to 6452b33 Compare April 14, 2016 11:46

Add librato integration

770f315

RafaelloLollipop force-pushed the librato-integration branch from 6452b33 to 770f315 Compare April 14, 2016 12:48

timabbott force-pushed the master branch from 9846caa to 35b0af2 Compare April 22, 2016 00:55

timabbott reviewed Apr 22, 2016
View reviewed changes

RafaelloLollipop closed this Jul 13, 2016

RafaelloLollipop reopened this Jul 13, 2016

TomaszKolek mentioned this pull request Sep 1, 2016

Add Librato integration #1749

Closed

timabbott closed this Sep 14, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Librato integration #648

Add Librato integration #648

RafaelloLollipop commented Apr 11, 2016

smarx commented Apr 11, 2016

timabbott Apr 11, 2016

timabbott commented Apr 11, 2016

timabbott commented Apr 11, 2016

timabbott commented Apr 11, 2016

RafaelloLollipop commented Apr 11, 2016

timabbott commented Apr 11, 2016

zack-ahdach commented Apr 12, 2016

timabbott commented Apr 12, 2016

RafaelloLollipop commented Apr 12, 2016

timabbott Apr 13, 2016

timabbott commented Apr 13, 2016

RafaelloLollipop commented Apr 13, 2016

RafaelloLollipop commented Apr 18, 2016

timabbott commented Apr 19, 2016 •

edited

Loading

RafaelloLollipop commented Apr 19, 2016 •

edited

Loading

timabbott commented Apr 22, 2016

timabbott Apr 22, 2016

timabbott commented Apr 22, 2016

timabbott commented May 10, 2016

RafaelloLollipop commented Jun 7, 2016

timabbott commented Jun 7, 2016

TomaszKolek commented Jun 7, 2016

TomaszKolek commented Jul 13, 2016 •

edited

Loading

RafaelloLollipop commented Jul 13, 2016

timabbott commented Jul 14, 2016

timabbott commented Sep 14, 2016


		<p><b>Congratulations! You're done!</b><br /> When a snapshot comes, you'll get a Zulip notification that looks like this:</p>

		<p><img class="screenshot" src="/static/images/integrations/librato/009.png" /></p>

Add Librato integration #648

Add Librato integration #648

Conversation

RafaelloLollipop commented Apr 11, 2016

smarx commented Apr 11, 2016

timabbott Apr 11, 2016

Choose a reason for hiding this comment

timabbott commented Apr 11, 2016

timabbott commented Apr 11, 2016

timabbott commented Apr 11, 2016

RafaelloLollipop commented Apr 11, 2016

timabbott commented Apr 11, 2016

zack-ahdach commented Apr 12, 2016

timabbott commented Apr 12, 2016

RafaelloLollipop commented Apr 12, 2016

timabbott Apr 13, 2016

Choose a reason for hiding this comment

timabbott commented Apr 13, 2016

RafaelloLollipop commented Apr 13, 2016

RafaelloLollipop commented Apr 18, 2016

timabbott commented Apr 19, 2016 • edited Loading

RafaelloLollipop commented Apr 19, 2016 • edited Loading

timabbott commented Apr 22, 2016

timabbott Apr 22, 2016

Choose a reason for hiding this comment

timabbott commented Apr 22, 2016

timabbott commented May 10, 2016

RafaelloLollipop commented Jun 7, 2016

timabbott commented Jun 7, 2016

TomaszKolek commented Jun 7, 2016

TomaszKolek commented Jul 13, 2016 • edited Loading

RafaelloLollipop commented Jul 13, 2016

timabbott commented Jul 14, 2016

timabbott commented Sep 14, 2016

timabbott commented Apr 19, 2016 •

edited

Loading

RafaelloLollipop commented Apr 19, 2016 •

edited

Loading

TomaszKolek commented Jul 13, 2016 •

edited

Loading