Skip to content
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

Add factory for creating DogStatsd sinks from a prexisting datadog client #67

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

bobbytables
Copy link

As of right now, the datadog metric sink does not use buffered flushes, which can cause a noticeable slowdown in emitting metrics. The DogStatsd package offers a buffered version of their client, but because the client is not exported from this sink, it's not possible to set it. This change allows you to pass in a client that you initialize.

For example, it would be nice to be able to use the buffered client like so:

package foo

import (
  "github.com/DataDog/datadog-go/statsd"
  "github.com/armon/go-metrics/datadog"
)

client, err := statsd.NewBuffered("localhost:8125", 1024)
if err != nil {
  panic(err)
}

sink := datadog.NewDogStatsdSinkFromClient(client, "hostname")

@bobbytables
Copy link
Author

Any thoughts on this?

@aantono
Copy link

aantono commented Jan 18, 2018

Would it be possible, as part of this PR, to also add a getter to the DogStatsD sync to expose the Client? Since the DogStatsD client also provides ability to send events, it would be nice to be able to just instantiate a single client instance, as part of the DogStatsD sync and reuse it externally for Event emitting purposes as well.

@hashicorp-cla
Copy link

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

Have you signed the CLA already but the status is still pending? Recheck it.

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.

3 participants