-
Notifications
You must be signed in to change notification settings - Fork 113
client: Add context parameter and enable tracing support #328
client: Add context parameter and enable tracing support #328
Conversation
Note that this change is somewhat "invasive" since it adds an opentracing dependency to the client. However, that's the direction we want to move in anyway (see kata-containers/kata-containers#27). Note also that the gRPC server the agent runs is not currently traced - that will be dealt with on #322. |
CI is not happy
|
Add a `context.Context` parameter to the client `NewAgentClient()` API and enable gRPC tracing if the specified context contains an opentracing span. Fixes kata-containers#327. Signed-off-by: James O. D. Hunt <[email protected]>
d78ac8f
to
6d26d61
Compare
Branch updated. |
Codecov Report
@@ Coverage Diff @@
## master #328 +/- ##
==========================================
+ Coverage 44.73% 44.92% +0.18%
==========================================
Files 15 15
Lines 2345 2393 +48
==========================================
+ Hits 1049 1075 +26
- Misses 1162 1176 +14
- Partials 134 142 +8 |
@@ -54,14 +56,28 @@ type dialer func(string, time.Duration) (net.Conn, error) | |||
// - unix://<unix socket path> | |||
// - vsock://<cid>:<port> | |||
// - <unix socket path> | |||
func NewAgentClient(sock string, enableYamux bool) (*AgentClient, error) { | |||
func NewAgentClient(ctx context.Context, sock string, enableYamux bool) (*AgentClient, error) { |
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.
why this change? the context is always Background()
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.
We want to be able to pass the context the runtime creates to virtcontainers/kata_agent.go
because the runtime's context will have the trace information stored inside it so with this change, when the runtime is executed, we'll get gRPC traces for the client (runtime) side.
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.
make sense, thanks
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
Add a
context.Context
parameter to the clientNewAgentClient()
API andenable gRPC tracing if the specified context contains an opentracing
span.
Fixes #327.
Signed-off-by: James O. D. Hunt [email protected]