search

DataDog / java-dogstatsd-client

Java statsd client library
MIT License
176 stars 101 forks source link

Add named pipe support for windows #169

Closed randomanderson closed 2 years ago

randomanderson commented 3 years ago

This PR adds named pipe support for windows machines. Named pipes are similar to unix domain sockets. This PR supports submitting metrics to Datadog in Azure App Services (https://github.com/DataDog/datadog-aas-extension).

The main changes are:

Unfortunately, only existing named pipes can be written to in pure Java. In order to create named pipes for testing, JNA is used.

randomanderson commented 2 years ago

@vickenty Did you want to rereview or should I just merge this PR with the fixes?

vickenty commented 2 years ago

@randomanderson Sorry for the delay, I will take another look. Meanwhile, we would like to make sure configuration options are the same between libraries for different languages. Windows pipes are not universally supported by all libraries we support, but where it is available, we'd like the configuration to be the same (same env vars / methods). Can you please check if we're aligned with https://github.com/DataDog/datadog-go for example?

randomanderson commented 2 years ago

@vickenty Windows pipes are only implemented in .NET and the dogstatsd server. They both use DD_DOGSTATSD_PIPE_NAME which is what I used here

vickenty commented 2 years ago

There are some warnings in the CI, would you mind taking a look and fixing if any are new?

Do you think it might be useful to throw an error if named pipe is requested on a non-windows OS?

randomanderson commented 2 years ago

@vickenty There were a few javadoc warnings that I fixed. The rest of the warnings were preexisting (one about the deploy plugin missing a version and one about the EnvironmentVariables test fixture)

There's no error for Unix Domain Sockets on Windows so I don't think there needs to be one for named pipes. If the named pipe does not exist, an IOException is thrown.

randomanderson commented 2 years ago

@vickenty I made that small change. I left the 2-arg constructor because UnixDatagramClientChannel uses it.