tracing: Remove trace mode and trace type #2353

jodh-intel · 2021-07-29T15:24:48Z

Remove the trace_mode and trace_type agent tracing options as
decided in the Architecture Committee meeting.

See:

docs: Add tracing proposals doc #2062

Fixes: #2352.

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

jodh-intel · 2021-07-29T15:25:11Z

Added dnm label as this still needs to be manually tested...

jodh-intel · 2021-07-30T13:43:18Z

Manual tests worked so please review so we can land y'all!

Related: #2363.

jodh-intel · 2021-07-30T14:55:06Z

/test

jodh-intel · 2021-08-09T12:55:25Z

/cc @liubin.

jodh-intel · 2021-08-09T13:44:54Z

/test

liubin · 2021-08-10T02:25:40Z

src/agent/src/config.rs

-            if let Ok(result) = value.parse::<tracer::TraceType>() {
-                self.tracing = result;
+        if let Ok(value) = env::var(TRACING_ENV_VAR) {
+            match value.as_str() {


Should use get_bool_value to parse the config value like command-line way?

Good catch @liubin! Updated.

liubin · 2021-08-10T02:33:04Z

And just a reminder that don't forget to update docs/how-to/how-to-set-sandbox-config-kata.md to remove these trace options. @jodh-intel

jodh-intel · 2021-08-10T08:31:47Z

Thanks @liubin - doc also updated.

jodh-intel · 2021-08-10T08:31:51Z

/test

cmaf

@jodh-intel, I tested this with tracing enabled for runtime and agent in the config file and see an error consistently:

ctr: Failed to check if grpc server is working: rpc error: code = DeadlineExceeded desc = timed out connecting to vsock 2900665692:1024: unknown

With only runtime tracing enabled, I am able run and trace with ctr. I also tested tracing with the latest from main and don't see this error with agent tracing configured. I verified that the host kernel was built with the CONFIG_VHOST_VSOCK option and that the module was loaded.

I don't see anything in your PR that explicitly modifies anything related to vsock...thoughts?

jodh-intel · 2021-08-24T14:26:11Z

/test

jodh-intel · 2021-08-27T14:22:13Z

Hi @cmaf - I've just re-tested on a fully up-to-date Ubuntu 20.04.3 system using Kata 2.2.0-rc0 + this PR with containerd v1.5.5 (72cec4be58a9eb6b2910f5d10f1c01ca47d231c0), and it wfm.

If you can still repeat this issue, can you revert to the main version of the code to definitively rule out the host kernel? It might be worth checking the behaviour on another system and checking the journal for vsock error messages (or stray processes?)

jodh-intel · 2021-08-27T14:29:09Z

@cmaf - wait a sec - are you using an image= or initrd=? If the latter, you may have hit #2443.

cmaf · 2021-08-31T15:04:30Z

@jodh-intel I tried a completely new install of 20.04 with the latest kata containers built from main, both containerd v1.5.2 and v1.5.5, and I can get tracing output for the agent. Both on bare metal and a VM. However, when I switch to this PR, I get the same error as above. Increasing the timeout did not fix it. I also verified that I am using image= in the configuration file.

I used the setup.sh script from the ci directory to install and configure, and further updated containerd to v1.5.5. Maybe I am missing a configuration step that you took?

Log output:

Aug 28 02:02:47 system-tracing-setup kata[108121]: time="2021-08-28T02:02:47.323236925Z" level=warning msg="Unable to connect to unix socket (/run/vc/vm/foo/qmp.sock): dial unix /run/vc/vm/foo/qmp.sock: connect: no such file or directory" name=containerd-shim-v2 pid=108121 sandbox=foo source=virtcontainers subsystem=qmp
Aug 28 02:02:47 system-tracing-setup kata[108121]: time="2021-08-28T02:02:47.323445025Z" level=error msg="Failed to connect to QEMU instance" error="dial unix /run/vc/vm/foo/qmp.sock: connect: no such file or directory" name=containerd-shim-v2 pid=108121 sandbox=foo source=virtcontainers subsystem=qemu
Aug 28 02:03:15 system-tracing-setup kata[108121]: time="2021-08-28T02:03:15.516250547Z" level=warning msg="sandbox cgroups path is empty" name=containerd-shim-v2 pid=108121 sandbox=foo source=virtcontainers subsystem=sandbox
Aug 28 02:03:15 system-tracing-setup kata[108121]: time="2021-08-28T02:03:15.519950348Z" level=warning msg="failed to cleanup netns" error="failed to get netns /var/run/netns/cnitest-c0ca549b-0de0-b343-426b-5c267956e6aa: failed to Statfs \"/var/run/netns/cnitest-c0ca549b-0de0-b343-426b-5c267956e6aa\": no such file or directory" name=containerd-shim-v2 path=/var/run/netns/cnitest-c0ca549b-0de0-b343-426b-5c267956e6aa pid=108121 sandbox=foo source=katautils

cmaf · 2021-08-31T18:51:14Z

I tested this with some changes to configuration.toml network settings, but cannot get output. When agent tracing is enabled, it looks the vm isn't being created.

cmaf

@jodh-intel I tested with agent debug enabled and saw an "invalid trace type" error followed by a shutdown of the agent:

Sep 15 19:33:18 test-system kata[175979]: time="2021-09-15T19:33:18.580312355Z" level=debug msg="reading guest console" console-protocol=unix console-url=/run/vc/vm/foo/console.sock name=containerd-shim-v2 pid=175979 sandbox=foo source=virtcontainers subsystem=sandbox vmconsole="Error: invalid trace type"
Sep 15 19:33:18 test-system kata[175979]: time="2021-09-15T19:33:18.583842736Z" level=debug msg="reading guest console" console-protocol=unix console-url=/run/vc/vm/foo/console.sock name=containerd-shim-v2 pid=175979 sandbox=foo source=virtcontainers subsystem=sandbox vmconsole="[\x1b[0;32m  OK  \x1b[0m] Stopped target \x1b[0;1;39mKata Containers Agent Target\x1b[0m."

The VM eventually shuts down as well. Please see code comments.

cmaf · 2021-09-15T21:28:02Z

src/agent/src/tracer.rs

@@ -56,7 +55,7 @@ impl FromStr for TraceType {
    }


L51-53:

"isolated" => Ok(TraceType::Isolated), "disabled" => Ok(TraceType::Disabled), _ => Err(TraceTypeError::new("invalid trace type")),

Shoud TraceType be removed and replaced with a boolean like it is elsewhere in the code?

FYI @jodh-intel on a second round of testing, I got the PR working with no error. However, the code referring to TraceType should probably still be removed.

jodh-intel · 2021-10-14T14:03:30Z

@cmaf - I've removed the TraceType from the tracer and updated the branch. Please tal so we can try to get this merged soon.

jodh-intel · 2021-10-14T14:03:38Z

/test

cmaf

@jodh-intel I tested it and it is working, but there is a static check failure:

[static-checks.sh:360] INFO: Running golangci-lint on /home/runner/work/kata-containers/kata-containers/src/github.com/kata-containers/kata-containers/src/runtime/cmd/kata-runtime
kata-env.go:153:14: fieldalignment: struct with 520 pointer bytes could be 512 (govet)
type EnvInfo struct {
             ^
make: *** [Makefile:35: static-checks] Error 1

Remove the `trace_mode` and `trace_type` agent tracing options as decided in the Architecture Committee meeting. See: - kata-containers#2062 Fixes: kata-containers#2352. Signed-off-by: James O. D. Hunt <[email protected]>

jodh-intel · 2021-10-15T09:13:00Z

@cmaf - fixed.

golangci-lint still doesn't show the suggested linter fix when it bleats. Until it does, the following are good places to look:

fix(runner): add suggested edit text from linter in display issue text golangci/golangci-lint#2135 (<-- worked for me)
Printing out SuggestedFixes golangci/golangci-lint#1778

jodh-intel · 2021-10-15T09:35:17Z

/test

jodh-intel added the do-not-merge PR has problems or depends on another label Jul 29, 2021

jodh-intel force-pushed the rm-trace-type-and-mode branch from b075d8d to 090cd8e Compare July 30, 2021 10:06

jodh-intel removed the do-not-merge PR has problems or depends on another label Jul 30, 2021

jodh-intel mentioned this pull request Jul 30, 2021

Make agent tracing require the forwarder to be running? #2363

Open

jodh-intel mentioned this pull request Jul 30, 2021

docs: Write tracing documentation #1937

Merged

jodh-intel force-pushed the rm-trace-type-and-mode branch from 090cd8e to 2a09dd0 Compare August 9, 2021 13:44

liubin reviewed Aug 10, 2021

View reviewed changes

liubin approved these changes Aug 10, 2021

View reviewed changes

jodh-intel force-pushed the rm-trace-type-and-mode branch from 2a09dd0 to 781aa92 Compare August 10, 2021 08:31

jodh-intel requested a review from a team as a code owner August 10, 2021 08:31

cmaf reviewed Aug 18, 2021

View reviewed changes

jodh-intel force-pushed the rm-trace-type-and-mode branch from 781aa92 to 07104ec Compare August 24, 2021 14:26

jodh-intel force-pushed the rm-trace-type-and-mode branch from 07104ec to d092cc2 Compare September 3, 2021 15:42

cmaf requested changes Sep 15, 2021

View reviewed changes

jodh-intel force-pushed the rm-trace-type-and-mode branch 2 times, most recently from c648901 to 8023d7e Compare October 14, 2021 14:02

cmaf approved these changes Oct 14, 2021

View reviewed changes

tracing: Remove trace mode and trace type

321be0f

Remove the `trace_mode` and `trace_type` agent tracing options as decided in the Architecture Committee meeting. See: - kata-containers#2062 Fixes: kata-containers#2352. Signed-off-by: James O. D. Hunt <[email protected]>

jodh-intel force-pushed the rm-trace-type-and-mode branch from 8023d7e to 321be0f Compare October 15, 2021 09:12

jodh-intel merged commit baf4784 into kata-containers:main Oct 15, 2021

cmaf mentioned this pull request Oct 18, 2021

Enable agent tracing for hybrid VSOCK hypervisors #2817

Merged

jodh-intel mentioned this pull request Nov 8, 2021

Remove StartTracing and StopTracing dynamic agent trace APIs #2985

Closed

cmaf mentioned this pull request Nov 9, 2021

runtime no longer honours trace_mode=static #413

Closed

cmaf mentioned this pull request Apr 20, 2022

agent shutdown test uses previously removed config option kata-containers/tests#4720

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tracing: Remove trace mode and trace type #2353

tracing: Remove trace mode and trace type #2353

jodh-intel commented Jul 29, 2021

jodh-intel commented Jul 29, 2021

jodh-intel commented Jul 30, 2021

jodh-intel commented Jul 30, 2021

jodh-intel commented Aug 9, 2021

jodh-intel commented Aug 9, 2021

liubin Aug 10, 2021

jodh-intel Aug 10, 2021

liubin commented Aug 10, 2021

jodh-intel commented Aug 10, 2021

jodh-intel commented Aug 10, 2021

cmaf left a comment

jodh-intel commented Aug 24, 2021

jodh-intel commented Aug 27, 2021

jodh-intel commented Aug 27, 2021

cmaf commented Aug 31, 2021 •

edited

Loading

cmaf commented Aug 31, 2021

cmaf left a comment

cmaf Sep 15, 2021

cmaf Sep 16, 2021

jodh-intel commented Oct 14, 2021

jodh-intel commented Oct 14, 2021

cmaf left a comment

jodh-intel commented Oct 15, 2021

jodh-intel commented Oct 15, 2021

tracing: Remove trace mode and trace type #2353

tracing: Remove trace mode and trace type #2353

Conversation

jodh-intel commented Jul 29, 2021

jodh-intel commented Jul 29, 2021

jodh-intel commented Jul 30, 2021

jodh-intel commented Jul 30, 2021

jodh-intel commented Aug 9, 2021

jodh-intel commented Aug 9, 2021

liubin Aug 10, 2021

Choose a reason for hiding this comment

jodh-intel Aug 10, 2021

Choose a reason for hiding this comment

liubin commented Aug 10, 2021

jodh-intel commented Aug 10, 2021

jodh-intel commented Aug 10, 2021

cmaf left a comment

Choose a reason for hiding this comment

jodh-intel commented Aug 24, 2021

jodh-intel commented Aug 27, 2021

jodh-intel commented Aug 27, 2021

cmaf commented Aug 31, 2021 • edited Loading

cmaf commented Aug 31, 2021

cmaf left a comment

Choose a reason for hiding this comment

cmaf Sep 15, 2021

Choose a reason for hiding this comment

cmaf Sep 16, 2021

Choose a reason for hiding this comment

jodh-intel commented Oct 14, 2021

jodh-intel commented Oct 14, 2021

cmaf left a comment

Choose a reason for hiding this comment

jodh-intel commented Oct 15, 2021

jodh-intel commented Oct 15, 2021

cmaf commented Aug 31, 2021 •

edited

Loading