Add virtcontainers trace support by jodh-intel · Pull Request #617 · kata-containers/runtime

jodh-intel · 2018-08-21T11:26:46Z

This PR expands the opentracing support in the runtime, providing increased granularity by adding trace spans to the virtcontainers packages.

Like the initial PR (#565), the strategy here is to add context.Context parameters to all virtcontainers API calls. The specified context replaces the default context created in certain parts of the code and allows trace spans to be created.

I've added a trace() function in cli/tracing.go but most subsystems and subsystem implementations also create their own trace() method.

Tags have been added to spans, mostly via the trace() functions:

component

This is equivalent to the source log field (see https://github.com/kata-containers/tests/tree/master/cmd/log-parser#logfile-requirements) and is named this way to conform to the standard
(https://github.com/opentracing/specification/blob/master/semantic_conventions.md#span-tags-table).
type

This is used to provide more information for interface implementations.

Examples:

For component=network, type could be cnm.
For component=hypervisor, type could be qemu.

Fixes #566.

Signed-off-by: James O. D. Hunt [email protected]

katacontainersbot · 2018-08-21T11:43:25Z

PSS Measurement:
Qemu: 167416 KB
Proxy: 4254 KB
Shim: 8867 KB

Memory inside container:
Total Memory: 2043464 KB
Free Memory: 2003596 KB

codecov · 2018-08-21T12:15:50Z

Codecov Report

Merging #617 into master will increase coverage by 0.46%.
The diff coverage is 77.74%.

@@            Coverage Diff             @@
##           master     #617      +/-   ##
==========================================
+ Coverage   64.69%   65.16%   +0.46%     
==========================================
  Files          84       84              
  Lines        9558     9835     +277     
==========================================
+ Hits         6184     6409     +225     
- Misses       2726     2769      +43     
- Partials      648      657       +9

opendev-zuul · 2018-08-21T12:28:20Z

Build failed (third-party-check pipeline) integration testing with
OpenStack. For information on how to proceed, see
http://docs.openstack.org/infra/manual/developers.html#automated-testing

kata-runsh : TIMED_OUT in 1h 01m 13s

jodh-intel · 2018-08-21T13:49:23Z

Given the amount of code this touches, it would be great to get this reviewed sooner rather than later to avoid "rebase hell" if code changes underneath this PR... :)

devimc · 2018-08-21T14:38:01Z

cli/create_test.go

 	}

-	testingImpl.CreateSandboxFunc = func(sandboxConfig vc.SandboxConfig) (vc.VCSandbox, error) {
+	testingImpl.CreateSandboxFunc = func(ctx context.Context, sandboxConfig vc.SandboxConfig) (vc.VCSandbox, error) {


I wonder if wouldn't be better to define a global context variable instead of modify all the functions

might be something like traceContext ..

I did think about that (and in fact did use that technique in virtcontainers/hack/virtc/main.go), but I'd rather we use the recommended approach and minimise globals where possible.

make sense, thanks for the info

devimc

lgtm

devimc · 2018-08-21T15:16:33Z

recheck

opendev-zuul · 2018-08-21T16:07:43Z

Build failed (third-party-check pipeline) integration testing with
OpenStack. For information on how to proceed, see
http://docs.openstack.org/infra/manual/developers.html#automated-testing

kata-runsh : NODE_FAILURE

jodh-intel · 2018-08-21T16:14:19Z

Ping @amshinde, @sboeuf, @bergwolf.

jodh-intel · 2018-08-21T16:53:00Z

Ping @egernst.

Set a tag on the root span to denote the subsystem refers to the runtime. Signed-off-by: James O. D. Hunt <[email protected]>

Simplify code slightly be creating a `trace()` function. Signed-off-by: James O. D. Hunt <[email protected]>

Add a `context.Context` parameter to all the virtcontainers API's to support tracing. Signed-off-by: James O. D. Hunt <[email protected]>

Create spans for all `virtcontainers` API functions. Signed-off-by: James O. D. Hunt <[email protected]>

Use the `networkLogger()`, not the network-specific `cnmLogger()`. Signed-off-by: James O. D. Hunt <[email protected]>

Add additional `context.Context` parameters and `struct` fields to allow trace spans to be created by the `virtcontainers` internal functions, objects and sub-packages. Note that not every function is traced; we can add more traces as desired. Fixes kata-containers#566. Signed-off-by: James O. D. Hunt <[email protected]>

katacontainersbot · 2018-08-22T07:39:33Z

PSS Measurement:
Qemu: 165467 KB
Proxy: 4140 KB
Shim: 8913 KB

Memory inside container:
Total Memory: 2043464 KB
Free Memory: 2003564 KB

opendev-zuul · 2018-08-22T08:27:11Z

Build failed (third-party-check pipeline) integration testing with
OpenStack. For information on how to proceed, see
http://docs.openstack.org/infra/manual/developers.html#automated-testing

kata-runsh : TIMED_OUT in 1h 01m 02s

egernst · 2018-08-22T21:52:58Z

awesome. @jodh-intel to ease stable handling, do you think we should consider separating out the agent change from the VC changes? It makes it a bit easier to track a whole PR for applicability. WDYT?

bergwolf · 2018-08-23T06:48:05Z

@egernst Is this going to be a stable candidate?

jodh-intel · 2018-08-23T08:36:34Z

@egernst - I agree. I've removed that commit and will raise a follow-on PR when this PR gets merged...

@egernst, @bergwolf - Agreed - I don't think the tracing code should be in stable yet as it's brand new (and hence "unproven" at this point).

jodh-intel · 2018-08-23T08:36:59Z

Ping @kata-containers/runtime - please can you review??

opendev-zuul · 2018-08-23T09:03:11Z

Build failed (third-party-check pipeline) integration testing with
OpenStack. For information on how to proceed, see
http://docs.openstack.org/infra/manual/developers.html#automated-testing

kata-runsh : NODE_FAILURE

katacontainersbot · 2018-08-23T09:09:18Z

PSS Measurement:
Qemu: 169712 KB
Proxy: 4089 KB
Shim: 8790 KB

Memory inside container:
Total Memory: 2043464 KB
Free Memory: 2003564 KB

jodh-intel · 2018-08-23T15:45:04Z

Errm, checking the F27 CI log:

Repository  was already defined in a 'Depends-on:' tag.
Only one repository per tag is allowed.

WTF? I didn't add any depends-on...? That message looks like "repository" is actually "" (note the double space before "was").

/me puts another coin in the slot and pulls the handle...

jodh-intel · 2018-08-23T15:46:20Z

@amshinde, @bergwolf, @sboeuf, @egernst, @WeiZhang555 - ptal....

devimc · 2018-08-23T17:05:08Z

recheck

devimc · 2018-08-23T17:05:22Z

restarting jenkins-ci-fedora-27-vsocks

raravena80 · 2018-12-17T15:24:26Z

Talked about in 12/17/18 meeting (kata-herding)

`dep check` is being run by our CI to make sure that the dependency files and vendor repositories are in sync. This PR brings them into sync to pass checks. Fixes: kata-containers#617 Signed-off-by: Ganesh Maharaj Mahalingam <[email protected]>

egernst added the review label Aug 21, 2018

jodh-intel mentioned this pull request Aug 21, 2018

Adding OpenTracing support for Kata kata-containers/kata-containers#27

Closed

jodh-intel requested review from bergwolf and sboeuf and removed request for bergwolf August 21, 2018 13:43

devimc reviewed Aug 21, 2018

View reviewed changes

devimc approved these changes Aug 21, 2018

View reviewed changes

James O. D. Hunt added 6 commits August 22, 2018 08:24

tracing: Add subsystem tag to runtime

a193366

Set a tag on the root span to denote the subsystem refers to the runtime. Signed-off-by: James O. D. Hunt <[email protected]>

tracing: Create trace function

f0073be

Simplify code slightly be creating a `trace()` function. Signed-off-by: James O. D. Hunt <[email protected]>

tracing: Add context to virtcontainers API

c200b28

Add a `context.Context` parameter to all the virtcontainers API's to support tracing. Signed-off-by: James O. D. Hunt <[email protected]>

tracing: Add trace spans to virtcontainers APIs

90970d9

Create spans for all `virtcontainers` API functions. Signed-off-by: James O. D. Hunt <[email protected]>

network: Use correct logger

6ddc9b4

Use the `networkLogger()`, not the network-specific `cnmLogger()`. Signed-off-by: James O. D. Hunt <[email protected]>

jodh-intel force-pushed the add-vc-trace-support branch from 18755b9 to bc83fb1 Compare August 22, 2018 07:25

jodh-intel mentioned this pull request Aug 22, 2018

network: Move OCI hooks and network namespace creation out of virtcontainers #623

Merged

jodh-intel force-pushed the add-vc-trace-support branch from bc83fb1 to d0679a6 Compare August 23, 2018 08:34

jodh-intel requested review from WeiZhang555, amshinde, bergwolf and sboeuf and removed request for sboeuf August 23, 2018 16:41

egernst approved these changes Aug 23, 2018

View reviewed changes

sboeuf approved these changes Aug 23, 2018

View reviewed changes

egernst merged commit 3f45818 into kata-containers:master Aug 23, 2018

egernst removed the review label Aug 23, 2018

This was referenced Aug 24, 2018

Add gRPC tracing #640

Closed

Implement containerd shim v2 API for Kata Containers #572

Merged

sboeuf added the feature New functionality label Sep 12, 2018

Conversation

jodh-intel commented Aug 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

katacontainersbot commented Aug 21, 2018

Uh oh!

codecov bot commented Aug 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

opendev-zuul bot commented Aug 21, 2018

Uh oh!

jodh-intel commented Aug 21, 2018

Uh oh!

devimc Aug 21, 2018

Choose a reason for hiding this comment

Uh oh!

devimc Aug 21, 2018

Choose a reason for hiding this comment

Uh oh!

jodh-intel Aug 21, 2018

Choose a reason for hiding this comment

Uh oh!

devimc Aug 21, 2018

Choose a reason for hiding this comment

Uh oh!

devimc left a comment

Choose a reason for hiding this comment

Uh oh!

devimc commented Aug 21, 2018

Uh oh!

opendev-zuul bot commented Aug 21, 2018

Uh oh!

jodh-intel commented Aug 21, 2018

Uh oh!

jodh-intel commented Aug 21, 2018

Uh oh!

katacontainersbot commented Aug 22, 2018

Uh oh!

opendev-zuul bot commented Aug 22, 2018

Uh oh!

egernst commented Aug 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bergwolf commented Aug 23, 2018

Uh oh!

jodh-intel commented Aug 23, 2018

Uh oh!

jodh-intel commented Aug 23, 2018

Uh oh!

opendev-zuul bot commented Aug 23, 2018

Uh oh!

katacontainersbot commented Aug 23, 2018

Uh oh!

jodh-intel commented Aug 23, 2018

Uh oh!

jodh-intel commented Aug 23, 2018

Uh oh!

devimc commented Aug 23, 2018

Uh oh!

devimc commented Aug 23, 2018

Uh oh!

raravena80 commented Dec 17, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

jodh-intel commented Aug 21, 2018 •

edited

Loading

codecov bot commented Aug 21, 2018 •

edited

Loading

egernst commented Aug 22, 2018 •

edited

Loading