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

expose a few connection stats #138

Merged
merged 4 commits into from
Aug 3, 2024
Merged

expose a few connection stats #138

merged 4 commits into from
Aug 3, 2024

Conversation

DaytonG
Copy link

@DaytonG DaytonG commented Jul 17, 2024

My company currently uses go-stomp in a sidecar attached to a main service. We've seen some deadlocks during normal operation, and wanted to monitor a few internal states within go-stomp. This change adds a simple struct defining the sizes of the read and write channels in a Connection object, as well as a few super basic counters. Our software would periodically call Connection.Stats() and export the data to our metrics systems for further monitoring.

@@ -8,7 +8,7 @@ jobs:
test:
strategy:
matrix:
go-version: [ 1.13.x, 1.x ]
go-version: [ 1.15.x, 1.x ]
Copy link
Author

Choose a reason for hiding this comment

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

I moved this up because the go.mod also has 1.15. Figured that should match.

Copy link
Collaborator

@worg worg left a comment

Choose a reason for hiding this comment

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

Can we make the stat collection optional?

@DaytonG
Copy link
Author

DaytonG commented Jul 30, 2024

I can take out the counters, then the stats would have the main part I care about (the size of the channels), which would only ever be fetched on request. Would that be ok?

@worg
Copy link
Collaborator

worg commented Jul 31, 2024

I can take out the counters, then the stats would have the main part I care about (the size of the channels), which would only ever be fetched on request. Would that be ok?

yeah, that works, I'm not against the counters, my thinking is to have an internal boolean field in the Conn enabled via a WithStats functional option, so the count only happens for those that explicitly request them

@DaytonG
Copy link
Author

DaytonG commented Jul 31, 2024

Alrighty! In an effort to keep the counters in, I added a boolean around the increments

@worg worg merged commit 9a13a1a into go-stomp:master Aug 3, 2024
2 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.

2 participants