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

refactor(statsd receiver): rename Statsd.UDPlistener => UDPConn #16420

Closed
wants to merge 2 commits into from

Conversation

Manbeardo
Copy link
Contributor

@Manbeardo Manbeardo commented Jan 21, 2025

Summary:

Later in this stack of diffs, I rename TCPlistener to Listener because it becomes a generic listener for all networks supported by net.Listen. That makes UDPlistener's naming stand out because:

  • It isn't a listener. It's a *net.UDPConn.
  • It isn't the UDP counterpart to Listener.

This diff renames it to UDPConn to improve clarity.

Test Plan:

Unit tests pass.

Stack created with Sapling. Best reviewed with ReviewStack.

@telegraf-tiger
Copy link
Contributor

Thanks so much for the pull request!
🤝 ✒️ Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla

James Ribe added 2 commits January 21, 2025 14:27
…ruct

Summary:

Later in this stack of diffs, I add a field named `MaxConnections` to `Statsd` because it controls the max number of connections for all listener types, not just TCP listeners. That creates a naming collision with the existing field named `MaxConnections`, which is an internal stat tracker. This diff eliminates that name collision by encapsulating all of `Statsd`'s internal stat trackers in a struct.

Test Plan:

Existing unit tests pass.
Summary:

Later in this stack of diffs, I rename `TCPlistener` to `Listener` because it becomes a generic listener for all networks supported by `net.Listen`. That makes `UDPlistener`'s naming stand out because:

- It isn't a listener. It's a `*net.UDPConn`.
- It isn't the UDP counterpart to `Listener`.

This diff renames it to `UDPConn` to improve clarity.

Test Plan:

Unit tests pass.
@Manbeardo Manbeardo changed the title [statsd receiver] rename Statsd.UDPlistener => UDPConn refactor(statsd receiver): rename Statsd.UDPlistener => UDPConn Jan 21, 2025
@Manbeardo
Copy link
Contributor Author

!signed-cla

@telegraf-tiger
Copy link
Contributor

Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

@Manbeardo this PR does two things it renames the UDPlistener and introduces a stats substructure. Please split the PR into two separate things!

@srebhan srebhan self-assigned this Jan 31, 2025
@Manbeardo
Copy link
Contributor Author

Manbeardo commented Feb 1, 2025

This PR is stacked on top of #16419. I'm not sure why GitHub hasn't updated the diff, but the stats stuff has already been merged.

@srebhan
Copy link
Member

srebhan commented Feb 12, 2025

@Manbeardo you need to rebase this PR on the latest master after #16419 is in to get rid of the additional diff...

@srebhan srebhan added the waiting for response waiting for response from contributor label Feb 12, 2025
@telegraf-tiger
Copy link
Contributor

Hello! I am closing this issue due to inactivity. I hope you were able to resolve your problem, if not please try posting this question in our Community Slack or Community Forums or provide additional details in this issue and reqeust that it be re-opened. Thank you!

@telegraf-tiger telegraf-tiger bot closed this Feb 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for response waiting for response from contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants