obfuscurity / tasseo

Live dashboard for Graphite
Other
1.53k stars 127 forks source link

Add AWS cloudwatch datasource #91

Closed ilyakava closed 10 years ago

ilyakava commented 10 years ago

Adds a new AWS CloudWatch datasource for metrics, using the JS aws sdk.

dmourati commented 10 years ago

Cool!

Also consider adding Cloudwatch Logs: http://aws.amazon.com/about-aws/whats-new/2014/07/10/introducing-amazon-cloudwatch-logs/

obfuscurity commented 10 years ago

@ilyakava Cool stuff, but where are the documentation patches? :smile_cat:

ilyakava commented 10 years ago

Right you are @obfuscurity! Added an extra commit with the documentation.

ilyakava commented 10 years ago

@obfuscurity anything else I should do before it's merge able?

obfuscurity commented 10 years ago

Sorry @ilyakava it's been busy lately. :smile_cat:

I think this is awesome but I haven't personally used the CloudWatch API so I can't quickly test this myself.

@gorsuch @eric or @roidrage - Is this something you'd be able to quickly review and test yourselves?

ilyakava commented 10 years ago

@obfuscurity would you like me to write a spec for the datasource with some stubbed VCR responses from CloudWatch pre-merge?

gorsuch commented 10 years ago

Hiya! Sorry, I missed this notification. Awesome me.

I'll give this a test today and update here.

obfuscurity commented 10 years ago

@gorsuch bump

ilyakava commented 10 years ago

I think the best I could do specs wise is to stub out the aws sdk, and enforce that the correct request params are built and that a given response is parsed correctly into an array of points.

Not very juicy, and it would mean the first mocha specs. Should I go ahead and do this?

obfuscurity commented 10 years ago

Actually, I think I'm more comfortable merging it in now given my recent experiences with CloudWatch. I just have one last question, will add it inline.

gorsuch commented 10 years ago

sorry for the horrible lateness here. this seems to be working as I'd expect. - +1

ilyakava commented 10 years ago

@gorsuch thanks for checking! :+1:

@obfuscurity I delayed the script loading by checking the ENV var that would need to be set for cloudwatch. I also liked your var usingCloudWatch = true; suggestion since selecting the datasource in the metric configuration will allow having multiple sources per tasseo deployment (before if both graphite and cloudwatch were being used, the cloudwatch ENV var would have precluded graphite dashboards from ever getting the correct datasource)

good to go?

obfuscurity commented 10 years ago

Yeah, that works too. A lot cleaner than my suggestion. :+1: :smile_cat: