-
Notifications
You must be signed in to change notification settings - Fork 45
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
RSDK-8819: Instrument some grpc/webrtc FTDC stats. #385
Conversation
rpc/wrtc_server.go
Outdated
TotalTimeConnectingMillis: srv.counters.TotalTimeConnectingMillis.Load(), | ||
} | ||
ret.PeersActive = ret.PeersConnected - ret.PeersDisconnected | ||
ret.AverageTimeConnectingMillis = float64(ret.TotalTimeConnectingMillis) / float64(ret.PeersConnected) |
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.
The graph for this was all zeroes. The values this is derived from looked good so I need to figure out what happened here.
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.
Figured this out. Because PeersConnected
starts at 0, golang assigns NaN
to AverageTimeConnectingMillis
.
And the ftdc format did not see that going from nan -> y(es)an as a "diff".
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.
Nice! Some questions (let me know if I'm going too deep on this stuff and you just want to get something in).
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.
LGTM! Thanks.
There are some (minimal) RDK changes to get this hooked up. Here's an example of how these stats show up.
edit Oops, the "web service" was already a component. So we get double stats in that image!