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 initial UDS stream implementation #228

Merged
merged 24 commits into from
Jan 23, 2024

Conversation

iksaif
Copy link
Contributor

@iksaif iksaif commented Nov 28, 2023

This PR adds experimental support for UDS streams, which can be used by using DD_DOGSTATSD_URL=unixstream:///path/to/dsd.sock or the following code:

    Callable<SocketAddress> addressLookup = new Callable<SocketAddress>() {
            @Override
            public SocketAddress call() throws Exception {
                return new UnixSocketAddressWithTransport(new UnixSocketAddress("/path/to/dsd.sock"), UnixSocketAddressWithTransport.TransportType.UDS_STREAM));
            }
        };

        client = new NonBlockingStatsDClientBuilder().prefix("my.prefix")
            .addressLookup(addressLookup)
            .port(0)
            .build();

Before this transport becomes GA we should revisit the builder to make it easier to use.

The telemetry will behave exactly like the go client with DataDog/datadog-go#290
If unix:// is used or TransportType.UDS we still default to SOCK_DGRAM and do not try to guess the socket type (yet) as this isn't trivial with the current code structure (in particular when we create the client before the socket exists)

@iksaif iksaif marked this pull request as ready for review November 29, 2023 09:29
@iksaif iksaif requested review from a team as code owners November 29, 2023 09:29
@blt
Copy link

blt commented Dec 5, 2023

I find it a little strange that none of the gitlab CI appears to have fired. Worthwhile to push an empty commit, or rebase from main, in case gitlab was broken at the branch point of this PR.

@iksaif
Copy link
Contributor Author

iksaif commented Dec 6, 2023

I find it a little strange that none of the gitlab CI appears to have fired. Worthwhile to push an empty commit, or rebase from main, in case gitlab was broken at the branch point of this PR.

We don't have gitlab CI for this project

iksaif added a commit to DataDog/datadog-go that referenced this pull request Jan 2, 2024
src/test/java/com/timgroup/statsd/UnixSocketTest.java Outdated Show resolved Hide resolved
src/test/java/com/timgroup/statsd/UnixSocketTest.java Outdated Show resolved Hide resolved
src/test/java/com/timgroup/statsd/UnixSocketTest.java Outdated Show resolved Hide resolved
src/test/java/com/timgroup/statsd/UnixSocketTest.java Outdated Show resolved Hide resolved
src/test/java/com/timgroup/statsd/UnixSocketTest.java Outdated Show resolved Hide resolved
src/test/java/com/timgroup/statsd/UnixSocketTest.java Outdated Show resolved Hide resolved
@iksaif
Copy link
Contributor Author

iksaif commented Jan 12, 2024

I think I addressed everything but give me some time to validate that the tests are not flakky

iksaif added a commit to DataDog/datadog-go that referenced this pull request Jan 17, 2024
Copy link
Contributor

@vickenty vickenty left a comment

Choose a reason for hiding this comment

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

Small nits about comments, but otherwise LGTM

src/test/java/com/timgroup/statsd/UnixSocketTest.java Outdated Show resolved Hide resolved
src/test/java/com/timgroup/statsd/UnixSocketTest.java Outdated Show resolved Hide resolved
@iksaif iksaif merged commit f6219c8 into DataDog:master Jan 23, 2024
3 of 11 checks passed
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