Implement server and client completed_rpcs (#1214)
#1216
Conversation
…1214) The previous impl appears to be a copy/paste error that directed data into the latency metric, but never actually recorded data preserving measure accuracy. This cl implements the `completed_rpcs` measure by incrementing by 1 for each `handleRPCEnd` call. The measure is broken down by method and status.
There was a problem hiding this comment.
LGTM
@smhendrickson - can you please sign the CLA as per https://github.com/census-instrumentation/opencensus-go/blob/master/CONTRIBUTING.md?
There was a problem hiding this comment.
It was working as intended. When you use 'Count' aggregation on latency you basically get the number of rpcs completed. Count aggregation increases value by 1 for every latency measured.
Doing one less Record call is better than two. (probably not much saving)
|
@rghetia agreed. I've just checked my metrics and can confirm With that in mind I can either reduce this pr to an informative comment, or keep it as is. I find the PR more straightforward but it really depends how efficient you want the plugin to be. Up to you all. RE: CLA not sure why this isn't working. My @google email should get his to pass. Will keeping looking. |
I would go with an informative comment. |
This reuse previously threw us for a loop. We mistakenly thought it was a copy/paste error. census-instrumentation#1214 census-instrumentation#1216
|
#1217 created for the comment. Unsure if it is correct form to create a new PR for something like that, but thats what happens when I don't use github (or git) for years 🤷♂️ |
The previous implementation appears to be a copy-paste error that directed data into
the latency metric, but never recorded data preserving the latency measure
accuracy. This cl implements the
completed_rpcsmeasure byincrementing by 1 for each
handleRPCEndcall.The measure is broken down by method and status.