locustio / locust

Write scalable load tests in plain Python 🚗💨
MIT License
24.64k stars 2.96k forks source link

hooking locust and graphite together as a plugin #94

Closed sanga closed 11 years ago

sanga commented 11 years ago

we have lots of metrics being recorded to graphite and pushing locust data into the same graphite instance makes it really nice to compare how (for example) cpu/disk io/mem usage correlates with locust request metrics.

I had that metric-pushing-to-graphite running in a separate process that was polling locusts web api but that was quite clunky/not very reliable. So yesterday I got the pushing working directly from locust. At the moment that required a one-line patch to locust (but in a more polished form it would be bigger). Basically, I wanted some feedback on the method before going and polishing it for inclusion into locust (or then abandoning it for something better).

The guts is patching web.py to add a new event that we hook into, so that when the web app javascript calls /stats/requests that fires the event which then runs a func written in the client to push the json request stats json object up to graphite. That seems like an OK way to do it to me. It means for example that any other monitoring system could be added in a similar way just by writing a different event hook func. The unpolished bit at the moment is that that event is thrown at the end of the handler in the web app, which isn't the right place for it in my opinion, but fixing that would mean moving the logic out of the handler to somewhere else.

There has been some discussions about this kind of thing (graphite and plugins etc) in https://github.com/locustio/locust/issues/34. How do you feel about that approach? (apologies for the rather long explanation)

sanga commented 11 years ago

Same thing is code form:

https://github.com/sanga/locust/tree/add_stats_called_event_hook

not exactly how I'd do it, but you can see the idea, as a request for comment.

heyman commented 11 years ago

Hi!

It definitely sounds nice to be able to correlate the current locust stats, with cpu/mem/io in a good way, so I think your use-case is super valid. However I have two main issues with the proposal.

The first, and biggest, is that I don't think an event hook like that would be the appropriate place to put that kind of code, since the pushing of data (to graphite in this case) would be dependent on a browser looking at the web UI, and if two browsers were to have the same page loaded, twice the amount of data would be pushed (which I assume really wouldn't matter in your case, but still it seems wrong).

The second thing is that the /stats/ endpoints aren't currently a part of the public API, and therefore it just contains whatever has been needed for the web UI to work, and hasn't got the amount of consideration a public API deserves. This is however something that could definitely be fixed, I just want to be cautious when introducing new stuff to the API :).

I still think there could be a good reason for adding the event-hook that you are describing, in combination of adding a way for locust users to add functionality to the web UI. In that case, such event-hook could be used to attach extra data to the web UI, which custom client code could then do something with in the web UI.

In your case, maybe it would be better to be able to interface with the RequestStats API directly, and then you could spawn a separate greenlet that reads, and pushes, the appropiate data every X second?

Please note that I definitely think you have a valid use-case, and locust should support it in some way :).

sanga commented 11 years ago

Not to mention the most glaringly obvious (in retrospect ;) problem with doing it the way I suggested: no metrics will be published to graphite without the web ui running. That's a problem if you're running locust under jenkins, as we are.

Thanks for the help, Jonatan. Doing it the way you suggested was pretty trivial actually. A tribute to what a lovely tool you guys have produced.

The case you make for including the hook I was suggesting is actually something else I'll need at some point, but I'll save that one for another day.