Skip to content

add capability to also remove prefixes#7

Open
tiadobatima wants to merge 2 commits into
DataDog:masterfrom
tiadobatima:master
Open

add capability to also remove prefixes#7
tiadobatima wants to merge 2 commits into
DataDog:masterfrom
tiadobatima:master

Conversation

@tiadobatima

Copy link
Copy Markdown

No description provided.

@remh

remh commented Nov 18, 2015

Copy link
Copy Markdown

@tiadobatima thanks! Can you describe the use case ? Seems like a pretty edge case.
Thanks again!

@tiadobatima

Copy link
Copy Markdown
Author

Not sure if it's that much of an edge case: when metrics come from jmxtrans or any software that doesn't allow us to configure the prefix prior to sending to statsd, the metric will endup in datadog as:

server.hostname1.my.metric
server.hostname2.my.metric

Where server.hostname1, and server.hostname2 are the prefix added by jmxtrans depending on the host the metric is coming from, and my.metric is the actual metric.

And unless things changed, datadog doesn't allows us to aggregate different metric names as Graphite does. So, we have to send the metric as my.metric and tag it with hostname1, hostname2, and so on to be able to aggregate them.

We opened a ticket with you guys a while ago and were told that there wasn't any way to aggregate different metrics, and we cannot change every piece of software that generates metrics for us... So we thought the ability to remove prefixes before sending the metrics to datadog would be a nice way of dealing with that.

Cheers,
g.

@scalp42

scalp42 commented Dec 11, 2015

Copy link
Copy Markdown

@remh any chance to see this merged ? Indeed a lot of reporting tools do not allow the prefix to be changed (or "namespaced").

@remh

remh commented Jan 8, 2016

Copy link
Copy Markdown

@scalp42 can you rebase your branch against master ? There are some conflicts. I'll merge it after.
Thanks!

@tiadobatima

Copy link
Copy Markdown
Author

fixed the conflicts.

@tiadobatima

Copy link
Copy Markdown
Author

@remh ping :)

@tiadobatima

Copy link
Copy Markdown
Author

ping! :)

@scalp42

scalp42 commented Jun 27, 2019

Copy link
Copy Markdown

@remh can you look at it again please? 📆

@truthbk truthbk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am really ashamed of the time it took us to review this. My most sincere apologies. This looks almost ready to go to me @tiadobatima, I would perhaps change the config option a little bit to make it less confusing regarding its purpose.

Comment thread README.md
datadogApiKey: "your_api_key" // You can get it from this page: https://app.datadoghq.com/account/settings#api
datadogPrefix: "your_prefix" // Your metrics will be prefixed by this prefix
datadogTags: ["your:tag", "another:tag"] // Your metrics will include these tags
datadogRemovePrefix: 2 // Number of period delimited prefixes to remove. If you use this option with *datadogPrefix* remove will happen prior to addition.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this is a bit of a misleading name, maybe datadogMetricKeyTrim - so that the purpose of the variable is a little bit more clear in that it acts over the key, and that it's a trim operation?

@truthbk truthbk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: also might be nice to check the variable is indeed a number

Comment thread lib/datadog.js

var get_prefix = function datadog_get_prefix(key) {
var new_key = key;
if (datadogRemovePrefix !== undefined) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (datadogRemovePrefix !== undefined) {
if (datadogRemovePrefix !== undefined) && (typeof datadogRemovePrefix == "number") {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants