brettlangdon / node-dogapi

Datadog API Node.JS Client
https://brettlangdon.github.io/node-dogapi/
105 stars 45 forks source link

metric_type isn't doing anything? #35

Closed jetmind closed 8 years ago

jetmind commented 8 years ago

dogapi.metric.send takes metric_type as extra parameter and documentation specifies it can be either "gauge" or "counter", but setting it to "counter" works exactly as setting it to "gauge" (i.e. it reports constant value to the datadog).

I don't see any mentions of metric type at all in the Datadog's HTTP API docs and official API libraries (e.g. datadogpy) rely on the statsd to implement counters.

Maybe this option is obsolete? Or am I missing something?

Thanks!

brettlangdon commented 8 years ago

hey @jetmind thank you for reporting.

Yeah, it does appear as though metric_type isn't going to do anything, their docs specifically say that all metrics sent will be a gauge.

This is likely a change that I missed. I'll see if I can get around to removing metric_type from dogapi.metric.send and publishing a new version. Sorry about any confusion.

jetmind commented 8 years ago

Thanks for a prompt response!

Seems like the only way to get counters is to use statsd or write own aggregations manually now.

Thanks for clarifying it for me anyway

brettlangdon commented 8 years ago

Yeah, statsd is definitely the best way to send metrics to datadog anyways.

jetmind commented 8 years ago

After some experimentation now I see that it's possible to send counters like this:

› curl  -X POST -H "Content-type: application/json" \
-d "{ \"series\" :
         [{\"metric\":\"test.metric3\",
          \"points\":[[$(date +%s), 1]],
          \"type\":\"count\",
          \"host\":\"test.example.com\",
          \"tags\":[\"environment:test\"]}
        ]
    }" \
"https://app.datadoghq.com/api/v1/series?api_key=$DATADOG_API_KEY"

Note, parameter name is just type, not metric_type.

Not sure why this is not documented anywhere, but Ruby client does exactly this

So, maybe you could just change parameter name, instead of deleting it altogether.

brettlangdon commented 8 years ago

This is from the Python client: https://github.com/DataDog/datadogpy/blob/55bf806eef399e0f60204ede61a962e591c8ee5a/datadog/api/metrics.py#L88-L94

So it appears it used to allow metric_type, but no longer does.

This will be an easy change to make. Hold tight :)

brettlangdon commented 8 years ago

@jetmind this is fixed and pushed as v2.1.1, please update.

The fix was made in a backwards compatible way, so metric_type will go back to working as needed.

Thank you again for reporting, and investigating the proper fix for this issue!!!!!!

jetmind commented 8 years ago

Thanks for quick fix!

As you can see in my comment above, type's value should be "count" instead of "counter". I have submitted a pull request to change that in the docs.