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 ability to submit metrics via named pipes #3230

Merged
merged 7 commits into from
Jan 18, 2022

Conversation

randomanderson
Copy link
Contributor

@randomanderson randomanderson commented Dec 1, 2021

On Windows, allows metrics to be submitted via a named pipe. Most of the changes here are plumbing the named pipe config option through to the dogstatsd client builder.

I also added retry logic to the connection because there is often a race between the start of the dogstatsd server and the tracer starting.

This PR relies on DataDog/java-dogstatsd-client#169 being merged and released (does not compile until it is released)

There are no automated tests in this PR because currently neither the build, nor the testsuite can run on Windows. It would be quite a large effort to get the testsuite working on windows.

@randomanderson randomanderson added the tag: do not merge Do not merge changes label Dec 16, 2021
@randomanderson randomanderson marked this pull request as ready for review December 16, 2021 18:23
@randomanderson randomanderson requested a review from a team as a code owner December 16, 2021 18:23
Copy link
Contributor

@devinsba devinsba left a comment

Choose a reason for hiding this comment

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

Looks good to me, apparently CircleCI does have windows support, I think it would be great to get a sense for how many windows users we have, if it's a large enough proportion then we should prioritize getting tests working

@randomanderson
Copy link
Contributor Author

@devinsba It was fairly simple to add Windows to the JMXFetch PR: https://github.com/DataDog/java-dogstatsd-client/pull/169/files#diff-78a8a19706dbd2a4425dd72bdab0502ed7a2cef16365ab7030a5a0588927bf47 .

The work is really getting the build to work on Windows.

@randomanderson randomanderson removed the tag: do not merge Do not merge changes label Jan 12, 2022
@randomanderson randomanderson merged commit b9655b6 into master Jan 18, 2022
@randomanderson randomanderson deleted the landerson/named-pipe-statsd branch January 18, 2022 21:01
@github-actions github-actions bot added this to the 0.94.0 milestone Jan 18, 2022
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