Skip to content

fix(metriks): force empty Datadog sink hostname. Service name always in prefix. #140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 5, 2020

Conversation

smoya
Copy link
Contributor

@smoya smoya commented Aug 5, 2020

Name config field maps to Datadog sink hostname field. It is only used by
armon/go-metrics lib and in a really non-well documented way.

It is basically used for removing from the metrics key the hostname in
favor of adding it (manually by the user) as tag. The library creator
explains that it makes sense since Datadog is multidimensional so the
tags should be used instead.
This is causing issues like sending metrics without the expected prefix: I.e.:

  • Expected: my-app.hits
  • Got: hits

See https://github.com/armon/go-metrics/blob/master/datadog/dogstatsd.go#L63-L82

We totally disagree.

…in prefix.

`Name` maps to Datadog sink `hostname` field. It is only used by
armon/go-metrics lib and in a really non-well documented way.

It is basically used for removing from the metrics key the hostname in
favor of adding it (manually by the user) as tag. The library creator
explains that it makes sense since Datadog is multidimensional so the
tags should be used instead.

See https://github.com/armon/go-metrics/blob/master/datadog/dogstatsd.go#L63-L82

We totally disagree.
@smoya smoya requested a review from a team August 5, 2020 12:05
Copy link
Contributor

@mraerino mraerino left a comment

Choose a reason for hiding this comment

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

makes sense 👍

@smoya smoya added the type: chore work needed to keep the product and development running smoothly label Aug 5, 2020
@smoya smoya self-assigned this Aug 5, 2020
@smoya smoya requested a review from mheffner August 5, 2020 12:12
Copy link
Contributor

@mheffner mheffner left a comment

Choose a reason for hiding this comment

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

Yeah, I think that model is pretty common for single-dimension metric stores like Graphite.

This is an internal lib, so I'd say let's just remove it entirely to avoid confusion. Consumers can easily remove this single field from their configs when they update.

@smoya smoya requested a review from mheffner August 5, 2020 13:45
@smoya smoya force-pushed the fix/datadogSinkMetricsHostname branch from 514ac11 to 6571bb7 Compare August 5, 2020 13:47
Copy link
Contributor

@mheffner mheffner left a comment

Choose a reason for hiding this comment

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

:shipit:

@smoya smoya merged commit ff51327 into master Aug 5, 2020
@smoya smoya deleted the fix/datadogSinkMetricsHostname branch August 5, 2020 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: chore work needed to keep the product and development running smoothly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants