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

Reuse socket connection when sending metrics #49

Merged
merged 2 commits into from
Feb 19, 2018
Merged

Reuse socket connection when sending metrics #49

merged 2 commits into from
Feb 19, 2018

Conversation

vgarvardt
Copy link
Contributor

I use your library for statsd backend in our hellofresh/stats-php library. Recently we added quite a lot of new metrics to one of our legacy monolith project to profile some bottlenecks and found that although metrics are sent via UDP we got significant performance degradation. During the investigation we found that statsd opens new socket connection for every metric being sent, that is quite expensive operation.

I fixed the issue in our library - hellofresh/stats-php#3 (we tested it before merging), but I think it would be useful to have this feature/fix in the base library.

@marcqualie
Copy link
Member

Great catch @vgarvardt!

Everything seems to work and tests pass so I'm good with this. Clean abstraction to a function as well.

One tiny thing regarding the comments, could you possibly update to stick with the current format of comments that the rest of the files have for consistency?

Thanks
/Marc

@vgarvardt
Copy link
Contributor Author

@marcqualie sorry about that - applied autoformatting to the file with my default code style. Should be fixed now.

@marcqualie marcqualie merged commit 691de92 into thephpleague:master Feb 19, 2018
@marcqualie
Copy link
Member

Thanks for the great contribution @vgarvardt!

@vgarvardt vgarvardt deleted the feature/reuse-socket-connection branch February 19, 2018 21:37
@vgarvardt
Copy link
Contributor Author

@marcqualie what is the release cycle of the library? Is there estimated date when the patch will be available in the release version?

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.

2 participants