RSDK-9876: Add counts for each {component, gRPC method} invocation. #4765
Conversation
viambot
added
safe to test
dgottlieb
requested review from
abe-winter
and removed request for
abe-winter
viambot
added
safe to test
viambot
added
safe to test
| // Only count component APIs, for now. | ||
| var apiMethod string | ||
| switch { | ||
| case strings.HasPrefix(info.FullMethod, "/viam.component."): |
There was a problem hiding this comment.
is there a downside in adding services as well?
There was a problem hiding this comment.
Nope -- just didn't want to deal with double checking all of the kinds of method names to do string parsing on.
robot/web/web.go
Outdated
|
|
||
| // UnaryInterceptor returns an incoming server interceptor that will pull method information and | ||
| // optionally resource information to bump the request counters. | ||
| func (mc *RequestCounter) UnaryInterceptor( |
There was a problem hiding this comment.
should we add a stream interceptor as well?
There was a problem hiding this comment.
I think it's worthwhile, but not necessarily a blocker for this.
A stream being opened is an interesting event for sure, but we might care more about the messages being sent within a stream. And I think most stream calls are for webrtc dialing, which isn't covered by the viam.components prefix right now.
robot/web/web.go
Outdated
|
|
||
| // UnaryInterceptor returns an incoming server interceptor that will pull method information and | ||
| // optionally resource information to bump the request counters. | ||
| func (mc *RequestCounter) UnaryInterceptor( |
There was a problem hiding this comment.
Oops, thanks. Was originally calling this "MethodCounter" and waffled.
viambot
added
safe to test
viambot
added
safe to test
|
I think the request to add services + streaming are good ideas. But I don't think it blocks this PR. If you're feeling motivated, I'd gladly welcome additional contributors! |
No description provided.