Metrics API ignores multiple path arguments

[MichaelHierweck]

While the API docs (https://github.com/pyr/cyanite/blob/master/doc/source/api.rst#metrics) claim that the metrics API supports multiple path arguments only a single result is returned:

$ curl 'http://127.0.0.1:8080/metrics?path=com.example.h01.load.load.longterm&from=1490721538&to=1490807938'; echo; echo
{"from":1490804400,"to":1490807700,"step":300,"series":{"com.example.h01.load.load.longterm":[0.392,0.35649999999999993,0.35850000000000004,0.3370000000000001,0.31799999999999995,0.3610526315789474,0.3545,0.39849999999999997,0.3995,0.39449999999999996,0.36199999999999993,0.4545]}}

$ curl 'http://127.0.0.1:8080/metrics?path=com.example.h02.load.load.longterm&from=1490721538&to=1490807938'; echo; echo
{"from":1490804400,"to":1490807700,"step":300,"series":{"com.example.h02.load.load.longterm":[0.2733333333333334,0.25800000000000006,0.27349999999999997,0.28350000000000003,0.30850000000000005,0.27399999999999997,0.22650000000000006,0.201,0.17800000000000002,0.17950000000000005,0.1915,0.1795]}}

$ curl 'http://127.0.0.1:8080/metrics?path=com.example.h01.load.load.longterm&path=com.example.h02.load.load.longterm&=from=1490721538&to=1490807938'; echo; echo
{"from":1490804400,"to":1490807700,"step":300,"series":{"com.example.h01.load.load.longterm":[0.392,0.35649999999999993,0.35850000000000004,0.3370000000000001,0.31799999999999995,0.3610526315789474,0.3545,0.39849999999999997,0.3995,0.39449999999999996,0.36199999999999993,0.4545]}}

Multiple path arguments are used by graphite-api/cyanite when rendering multiple metrics into one chart.

[ifesdjeen]

As far as I know, according to HTTP spec, it has to be path[]. Full disclaimer: I haven't checked it on Cyanite yet (working on a different branch that's not fully ready just yet).

[MichaelHierweck]

path[] means http://127.0.0.1:8080/metrics?path=[com.example.h01.load.load.longterm,com.example.h02.load.load.longterm]&from=1490721538&to=1490807938'?

[ifesdjeen]

If I understand it correctly, it means that:

http://127.0.0.1:8080/metrics?path[]=com.example.h01.load.load.longterm&path[]=com.example.h02.load.load.longterm&from=1490721538&to=1490807938

[brutasse]

At least at some point in time ?path=foo&path=bar worked, because that's what graphite-cyanite does when making queries. path[] is a PHP thing (at least). The HTTP spec doesn't say anything about the way repeated query string parameters are parsed, it is up to the server to interpret them the way it needs (first wins, last wins, or yield a list).

[ifesdjeen]

Oh, I was sure it was a part of 3986 RFC, although I haven't checked. I only know that major web frameworks (like rails) did it this way, maybe they took it from php.

But if graphite-cyanite works this way, and it used to work, it's just a regression.

I'm mostly done with a branch I was working on and hope to be able to work on that one soon (unless someone wanted to tackle it right away).

[MichaelHierweck]

`http://127.0.0.1:8080/metrics?path[]=com.example.h01.load.load.longterm&path[]=com.example.h02.load.load.longterm&from=1490721538&to=14908079? => Internal Server Error

@brutasse: Yes... I tried to track down why graphite-web won't render multipe metrics into a single chart anymore and found out that Cyanite - in the current master version - only returns a single data series.

Either cyanite or graphite-cyanite should be fixed. I might fix graphite-cyanite if someone can tell me which parameter format Cyanite expects. (Please add examples to the API docs.)

[ifesdjeen]

I'm pretty sure it's on cyanite side. I just didn't know about this feature and it did't have any tests. I'll make sure to fix it and add a test if possible..

[brutasse]

@ifesdjeen thank you, that would be great :)

[mpenet]

If I recall, you need to use ring.middleware.params/wrap-params to get the ability to have either single value for a params or a vector of values decoded from the query-string into :params. It's a bit of a hairy thing tho: foo=bar will return {:foo "bar"} but foo=bar&foo=baz will return {:foo ["bar" baz"]}. Maybe cyanite used to use this middleware in the past?

[MichaelHierweck]

Thank you. I can confirm it works, when building cyanite against this PR.

[ifesdjeen]

Thank you @MichaelHierweck!