-
Notifications
You must be signed in to change notification settings - Fork 86
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 connections to stats (opened, closed, attempted) #174
Conversation
index.js
Outdated
this.connecting++ | ||
this._clientConnections++ | ||
let opened = false | ||
|
||
conn.on('open', () => { | ||
opened = true | ||
this.stats.connections.opened++ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should also track server connections ya?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: could also track client- and server connections separately, but I put them together for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now it's tracking them separately (cleaner that way)
index.js
Outdated
// The _handleServerConnectionSwam path above calls _handleServerConnection | ||
// again, so this is the moment where the conn is actually considered 'attempted' | ||
this.stats.connects.attempted++ | ||
conn.on('open', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sanity check: is the conn guaranteed to not yet be opened? (else I need to special-case that)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its always opened at this state so this event shouldnt be used, you use just increment it sync (its a server conn)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I removed the 'attempted' counter, and now always increment 'opened' here
Helps catch cases where connections are continuously closed and re-opened, and connections which never open or get aborted (
connections.attempted - connections.opened
)