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 Unix Domain Socket Listener to statsdreceiver #21385

Open
michaelli321 opened this issue May 2, 2023 · 15 comments
Open

Add Unix Domain Socket Listener to statsdreceiver #21385

michaelli321 opened this issue May 2, 2023 · 15 comments
Labels
discussion needed Community discussion needed enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed receiver/statsd statsd related issues

Comments

@michaelli321
Copy link

michaelli321 commented May 2, 2023

Component(s)

receiver/statsd

Is your feature request related to a problem? Please describe.

We're currently migrating workloads to use the OTEL Collector for metric collection. The datadog-agent supports ingesting StatsD metrics via a Unix Domain Socket.

Unix Domain Sockets allow you to establish the connection with a socket file, regardless of the IP of the Datadog Agent container. It also enables the following benefits:

  • Bypassing the networking stack brings a significant performance improvement for high traffic.
  • While UDP has no error handling, UDS allows the Agent to detect dropped packets and connection errors, while still allowing a non-blocking use.
  • DogStatsD can detect the container from which metrics originated and tag those metrics accordingly.

Are there plans to also support UDS through the statsdreceiver?

Describe the solution you'd like

Adding a UDS listener to the statsdreceiver

Describe alternatives you've considered

No response

Additional context

No response

@michaelli321 michaelli321 added enhancement New feature or request needs triage New item requiring triage labels May 2, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 2, 2023

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions github-actions bot added the receiver/statsd statsd related issues label May 2, 2023
@michaelli321
Copy link
Author

Actually, I see this was already kept in mind when writing the statsdreceiver: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/statsdreceiver/receiver.go#L79

@dmitryax
Copy link
Member

dmitryax commented May 2, 2023

Hi, @michaelli321, thanks for reporting. I don't think anyone is planning to work on this in the near future. Feel free to contribute if you have a chance.

@dmitryax dmitryax added help wanted Extra attention is needed good first issue Good for newcomers and removed needs triage New item requiring triage labels May 2, 2023
@g41797
Copy link
Contributor

g41797 commented May 5, 2023

Hi
As far as i understand you are talking about server side improvement
What about client?
Now I am in the middle of gavv/httpexpect#246
If this issue isn't urgent I'd like to participate

@g41797
Copy link
Contributor

g41797 commented May 6, 2023

/label needs-discussion

@atoulme atoulme added the discussion needed Community discussion needed label May 6, 2023
@g41797
Copy link
Contributor

g41797 commented May 6, 2023

Thanks

CONTRIBUTING.md: "needs discussion" vs "discussion need"

Run ./.github/workflows/scripts/add-labels.sh
failed to update #21385: 'needs discussion' not found
failed to update 1 issue
Error: Process completed with exit code 1.

@kang-makes
Copy link
Contributor

Hi!

I hope that I can have a PR to add this feature in a few days addressing this :)

@github-actions
Copy link
Contributor

github-actions bot commented Aug 9, 2023

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

  • issue: Github issue template generation code needs this to generate the corresponding labels.
  • receiver/statsd: @jmacd @dmitryax

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions github-actions bot added the Stale label Aug 9, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2023

This issue has been closed as inactive because it has been stale for 120 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 8, 2023
bogdandrutu pushed a commit that referenced this issue Dec 15, 2023
**Description:** 
This is the first step to adding UDS support to the StatsD receiver.

As I started to develop it, I saw that all tests were hardcoded to UDP
and using networking and there was no possibility to add a socket
communication.

PR started to get huge so I split it into two, one to refactor tests and
another one that adds UDS support:
 * Removed all unused references.
* Made a `Transport` helper that allows centralizing all supported
protocols and constants in its package.
* Removed all hardcoded UDP protocols and generalized testing so new
protocols are easy to add.

If you need a rationale about why the changes are like this, this is the
next PR I am going to submit after this one is merged:
kang-makes#2

That is the PR that is going to add UDS support properly.

**Link to tracking Issue:**
 - #21385

**Previous closed PR:**
 - #24832
cparkins pushed a commit to AmadeusITGroup/opentelemetry-collector-contrib that referenced this issue Jan 10, 2024
…28896)

**Description:** 
This is the first step to adding UDS support to the StatsD receiver.

As I started to develop it, I saw that all tests were hardcoded to UDP
and using networking and there was no possibility to add a socket
communication.

PR started to get huge so I split it into two, one to refactor tests and
another one that adds UDS support:
 * Removed all unused references.
* Made a `Transport` helper that allows centralizing all supported
protocols and constants in its package.
* Removed all hardcoded UDP protocols and generalized testing so new
protocols are easy to add.

If you need a rationale about why the changes are like this, this is the
next PR I am going to submit after this one is merged:
kang-makes#2

That is the PR that is going to add UDS support properly.

**Link to tracking Issue:**
 - open-telemetry#21385

**Previous closed PR:**
 - open-telemetry#24832
@jcogilvie
Copy link

@kang-makes any word on the PR to address this?

@michaelli321
Copy link
Author

michaelli321 commented Dec 1, 2024

I can open a PR for this in a week or two

@michaelli321
Copy link
Author

I started working on a draft here, but I noticed @kang-makes has a PR for this in their own fork already opened here.

@dmitryax dmitryax reopened this Dec 1, 2024
@kang-makes
Copy link
Contributor

kang-makes commented Dec 1, 2024

Hi @michaelli321 and @jcogilvie,

The PR is stale. At the time I wrote this, tests were coupled and had hardcoded responeses that needed a refactor before this one. Hence kang-makes#1

The refactor for tests was asked upstream (#24832), it was rebased 3 times and completely ignored even after going to the collector meetings begging for this to be merged.

The collector moved forward during this year and a half and the PR needs a full refactor from scratch (again).

I was moved out of the team in New Relic that maintains these integrations. In other circumstances I would keep working on it of community' sake but after the big amount of ungrateful work and complete silence from this project I am done.

I am done with this and I will not work on this project anymore.

@mx-psi
Copy link
Member

mx-psi commented Dec 11, 2024

@kang-makes Sorry that you had this experience, I can definitely see how that was frustrating. We are a relatively small group of contributors for the size of this project, and things move slowly and sometimes slip through the cracks, but either way we clearly failed here despite you doing the right thing and even coming to the meetings to try to move this forward.

I feel like since the time of the PR we have made small improvements on some things: on the Collector SIG we now have a dedicated CNCF Slack channel for Collector developers (#otel-collector-dev) where you can more easily ask for reviews and raise these problems without having to come to a meeting and we have grown the team a bit to have more people and spread the work, and at the project level we have a new SIG focused on Contributor Experience. But we still definitely have PRs that go without response or drag for too long, and we should at least be clear about the timelines and explicitly communicate that we are dropping things if we have to. Improving on this is the kind of work that is less 'shiny' and that both individuals and companies have less incentives to do, so progress has been slow, but I think it is one of the most important things that we have to get right, both as a SIG and also as a project.

If at some point you change your mind, I encourage you to both give another try to this particular issue as well as helping us brainstorm how to improve the contributor experience and prevent this in the future.

@jpkrohling
Copy link
Member

I just wanted to echo @mx-psi's word here: we failed here, and can do better. As a code owner who's had more components than I could handle and taking very long to review PRs for the components I own during very busy phases (conferences, off-sites, ...), I certainly feel guilty for this.

@kang-makes, I appreciate you giving us this feedback, this will hopefully help move things on our side. Even if it means only setting better expectations to our contributors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion needed Community discussion needed enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed receiver/statsd statsd related issues
Projects
None yet
Development

No branches or pull requests

8 participants