Skip to content

expose a few connection stats#138

Merged
worg merged 4 commits intogo-stomp:masterfrom
DaytonG:master
Aug 3, 2024
Merged

expose a few connection stats#138
worg merged 4 commits intogo-stomp:masterfrom
DaytonG:master

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.

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
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