Skip to content

Commit

Permalink
tracing: Remove trace mode and trace type
Browse files Browse the repository at this point in the history 
Remove the `trace_mode` and `trace_type` agent tracing options as
decided in the Architecture Committee meeting.

See:

- #2062

Fixes: #2352.

Signed-off-by: James O. D. Hunt <[email protected]>
  • Loading branch information
@jodh-intel
jodh-intel committed Aug 24, 2021
1 parent e26a140 commit 07104ec
Show file tree
Hide file tree
Showing 18 changed files with 148 additions and 400 deletions.
2 changes: 0 additions & 2 deletions docs/how-to/how-to-set-sandbox-config-kata.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ There are several kinds of Kata configurations and they are listed below.
| `io.katacontainers.config.agent.enable_tracing` | `boolean` | enable tracing for the agent |
| `io.katacontainers.config.agent.container_pipe_size` | uint32 | specify the size of the std(in/out) pipes created for containers |
| `io.katacontainers.config.agent.kernel_modules` | string | the list of kernel modules and their parameters that will be loaded in the guest kernel. Semicolon separated list of kernel modules and their parameters. These modules will be loaded in the guest kernel using `modprobe`(8). E.g., `e1000e InterruptThrottleRate=3000,3000,3000 EEE=1; i915 enable_ppgtt=0` |
| `io.katacontainers.config.agent.trace_mode` | string | the trace mode for the agent |
| `io.katacontainers.config.agent.trace_type` | string | the trace type for the agent |

## Hypervisor Options
| Key | Value Type | Comments |
Expand Down
196 changes: 89 additions & 107 deletions src/agent/src/config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
//
// SPDX-License-Identifier: Apache-2.0
//
use crate::tracer;
use anyhow::{bail, ensure, Context, Result};
use std::env;
use std::fs;
Expand All @@ -29,7 +28,7 @@ const VSOCK_PORT: u16 = 1024;
// Environment variables used for development and testing
const SERVER_ADDR_ENV_VAR: &str = "KATA_AGENT_SERVER_ADDR";
const LOG_LEVEL_ENV_VAR: &str = "KATA_AGENT_LOG_LEVEL";
const TRACE_TYPE_ENV_VAR: &str = "KATA_AGENT_TRACE_TYPE";
const TRACING_ENV_VAR: &str = "KATA_AGENT_TRACING";

const ERR_INVALID_LOG_LEVEL: &str = "invalid log level";
const ERR_INVALID_LOG_LEVEL_PARAM: &str = "invalid log level parameter";
Expand Down Expand Up @@ -58,7 +57,7 @@ pub struct AgentConfig {
pub container_pipe_size: i32,
pub server_addr: String,
pub unified_cgroup_hierarchy: bool,
pub tracing: tracer::TraceType,
pub tracing: bool,
}

// parse_cmdline_param parse commandline parameters.
Expand Down Expand Up @@ -103,7 +102,7 @@ impl AgentConfig {
container_pipe_size: DEFAULT_CONTAINER_PIPE_SIZE,
server_addr: format!("{}:{}", VSOCK_ADDR, VSOCK_PORT),
unified_cgroup_hierarchy: false,
tracing: tracer::TraceType::Disabled,
tracing: false,
}
}

Expand All @@ -119,11 +118,11 @@ impl AgentConfig {
// Support "bare" tracing option for backwards compatibility with
// Kata 1.x.
if param == &TRACE_MODE_OPTION {
self.tracing = tracer::TraceType::Isolated;
self.tracing = true;
continue;
}

parse_cmdline_param!(param, TRACE_MODE_OPTION, self.tracing, get_trace_type);
parse_cmdline_param!(param, TRACE_MODE_OPTION, self.tracing, get_bool_value);

// parse cmdline options
parse_cmdline_param!(param, LOG_LEVEL_OPTION, self.log_level, get_log_level);
Expand Down Expand Up @@ -183,10 +182,8 @@ impl AgentConfig {
}
}

if let Ok(value) = env::var(TRACE_TYPE_ENV_VAR) {
if let Ok(result) = value.parse::<tracer::TraceType>() {
self.tracing = result;
}
if let Ok(value) = env::var(TRACING_ENV_VAR) {
self.tracing = get_bool_value(&value)?;
}

Ok(())
Expand Down Expand Up @@ -236,25 +233,6 @@ fn get_log_level(param: &str) -> Result<slog::Level> {
logrus_to_slog_level(fields[1])
}

#[instrument]
fn get_trace_type(param: &str) -> Result<tracer::TraceType> {
ensure!(!param.is_empty(), "invalid trace type parameter");

let fields: Vec<&str> = param.split('=').collect();
ensure!(
fields[0] == TRACE_MODE_OPTION,
"invalid trace type key name"
);

if fields.len() == 1 {
return Ok(tracer::TraceType::Isolated);
}

let result = fields[1].parse::<tracer::TraceType>()?;

Ok(result)
}

#[instrument]
fn get_hotplug_timeout(param: &str) -> Result<time::Duration> {
let fields: Vec<&str> = param.split('=').collect();
Expand Down Expand Up @@ -339,10 +317,6 @@ mod tests {
use std::time;
use tempfile::tempdir;

const ERR_INVALID_TRACE_TYPE_PARAM: &str = "invalid trace type parameter";
const ERR_INVALID_TRACE_TYPE: &str = "invalid trace type";
const ERR_INVALID_TRACE_TYPE_KEY: &str = "invalid trace type key name";

// Parameters:
//
// 1: expected Result
Expand Down Expand Up @@ -393,7 +367,7 @@ mod tests {
container_pipe_size: i32,
server_addr: &'a str,
unified_cgroup_hierarchy: bool,
tracing: tracer::TraceType,
tracing: bool,
}

impl Default for TestData<'_> {
Expand All @@ -408,7 +382,7 @@ mod tests {
container_pipe_size: DEFAULT_CONTAINER_PIPE_SIZE,
server_addr: TEST_SERVER_ADDR,
unified_cgroup_hierarchy: false,
tracing: tracer::TraceType::Disabled,
tracing: false,
}
}
}
Expand Down Expand Up @@ -667,49 +641,115 @@ mod tests {
},
TestData {
contents: "trace",
tracing: tracer::TraceType::Disabled,
tracing: false,
..Default::default()
},
TestData {
contents: ".trace",
tracing: tracer::TraceType::Disabled,
tracing: false,
..Default::default()
},
TestData {
contents: "agent.tracer",
tracing: tracer::TraceType::Disabled,
tracing: false,
..Default::default()
},
TestData {
contents: "agent.trac",
tracing: tracer::TraceType::Disabled,
tracing: false,
..Default::default()
},
TestData {
contents: "agent.trace",
tracing: tracer::TraceType::Isolated,
tracing: true,
..Default::default()
},
TestData {
contents: "agent.trace=true",
tracing: true,
..Default::default()
},
TestData {
contents: "agent.trace=false",
tracing: false,
..Default::default()
},
TestData {
contents: "agent.trace=0",
tracing: false,
..Default::default()
},
TestData {
contents: "agent.trace=isolated",
tracing: tracer::TraceType::Isolated,
contents: "agent.trace=1",
tracing: true,
..Default::default()
},
TestData {
contents: "agent.trace=disabled",
tracing: tracer::TraceType::Disabled,
contents: "agent.trace=a",
tracing: false,
..Default::default()
},
TestData {
contents: "agent.trace=foo",
tracing: false,
..Default::default()
},
TestData {
contents: "agent.trace=.",
tracing: false,
..Default::default()
},
TestData {
contents: "agent.trace=,",
tracing: false,
..Default::default()
},
TestData {
contents: "",
env_vars: vec!["KATA_AGENT_TRACING="],
tracing: false,
..Default::default()
},
TestData {
contents: "",
env_vars: vec!["KATA_AGENT_TRACING=''"],
tracing: false,
..Default::default()
},
TestData {
contents: "",
env_vars: vec!["KATA_AGENT_TRACING=0"],
tracing: false,
..Default::default()
},
TestData {
contents: "",
env_vars: vec!["KATA_AGENT_TRACING=."],
tracing: false,
..Default::default()
},
TestData {
contents: "",
env_vars: vec!["KATA_AGENT_TRACE_TYPE=isolated"],
tracing: tracer::TraceType::Isolated,
env_vars: vec!["KATA_AGENT_TRACING=,"],
tracing: false,
..Default::default()
},
TestData {
contents: "",
env_vars: vec!["KATA_AGENT_TRACE_TYPE=disabled"],
tracing: tracer::TraceType::Disabled,
env_vars: vec!["KATA_AGENT_TRACING=foo"],
tracing: false,
..Default::default()
},
TestData {
contents: "",
env_vars: vec!["KATA_AGENT_TRACING=1"],
tracing: true,
..Default::default()
},
TestData {
contents: "",
env_vars: vec!["KATA_AGENT_TRACING=true"],
tracing: true,
..Default::default()
},
];
Expand Down Expand Up @@ -765,7 +805,7 @@ mod tests {
);
assert_eq!(config.container_pipe_size, 0, "{}", msg);
assert_eq!(config.server_addr, TEST_SERVER_ADDR, "{}", msg);
assert_eq!(config.tracing, tracer::TraceType::Disabled, "{}", msg);
assert_eq!(config.tracing, false, "{}", msg);

let result = config.parse_cmdline(filename);
assert!(result.is_ok(), "{}", msg);
Expand Down Expand Up @@ -1218,62 +1258,4 @@ Caused by:
assert_result!(d.result, result, msg);
}
}

#[test]
fn test_get_trace_type() {
#[derive(Debug)]
struct TestData<'a> {
param: &'a str,
result: Result<tracer::TraceType>,
}

let tests = &[
TestData {
param: "",
result: Err(anyhow!(ERR_INVALID_TRACE_TYPE_PARAM)),
},
TestData {
param: "agent.tracer",
result: Err(anyhow!(ERR_INVALID_TRACE_TYPE_KEY)),
},
TestData {
param: "agent.trac",
result: Err(anyhow!(ERR_INVALID_TRACE_TYPE_KEY)),
},
TestData {
param: "agent.trace=",
result: Err(anyhow!(ERR_INVALID_TRACE_TYPE)),
},
TestData {
param: "agent.trace==",
result: Err(anyhow!(ERR_INVALID_TRACE_TYPE)),
},
TestData {
param: "agent.trace=foo",
result: Err(anyhow!(ERR_INVALID_TRACE_TYPE)),
},
TestData {
param: "agent.trace",
result: Ok(tracer::TraceType::Isolated),
},
TestData {
param: "agent.trace=isolated",
result: Ok(tracer::TraceType::Isolated),
},
TestData {
param: "agent.trace=disabled",
result: Ok(tracer::TraceType::Disabled),
},
];

for (i, d) in tests.iter().enumerate() {
let msg = format!("test[{}]: {:?}", i, d);

let result = get_trace_type(d.param);

let msg = format!("{}: result: {:?}", msg, result);

assert_result!(d.result, result, msg);
}
}
}
6 changes: 3 additions & 3 deletions src/agent/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,8 @@ async fn real_main() -> std::result::Result<(), Box<dyn std::error::Error>> {
ttrpc_log_guard = Ok(slog_stdlog::init().map_err(|e| e)?);
}

if config.tracing != tracer::TraceType::Disabled {
let _ = tracer::setup_tracing(NAME, &logger, &config)?;
if config.tracing {
tracer::setup_tracing(NAME, &logger)?;
}

let root = span!(tracing::Level::TRACE, "root-span", work_units = 2);
Expand Down Expand Up @@ -244,7 +244,7 @@ async fn real_main() -> std::result::Result<(), Box<dyn std::error::Error>> {
}
}

if config.tracing != tracer::TraceType::Disabled {
if config.tracing {
tracer::end_tracing();
}

Expand Down
3 changes: 1 addition & 2 deletions src/agent/src/tracer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// SPDX-License-Identifier: Apache-2.0
//

use crate::config::AgentConfig;
use anyhow::Result;
use opentelemetry::sdk::propagation::TraceContextPropagator;
use opentelemetry::{global, sdk::trace::Config, trace::TracerProvider};
Expand Down Expand Up @@ -56,7 +55,7 @@ impl FromStr for TraceType {
}
}

pub fn setup_tracing(name: &'static str, logger: &Logger, _agent_cfg: &AgentConfig) -> Result<()> {
pub fn setup_tracing(name: &'static str, logger: &Logger) -> Result<()> {
let logger = logger.new(o!("subsystem" => "vsock-tracer"));

let exporter = vsock_exporter::Exporter::builder()
Expand Down
17 changes: 5 additions & 12 deletions src/runtime/cli/config/configuration-acrn.toml.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,24 +124,17 @@ block_device_driver = "@DEFBLOCKSTORAGEDRIVER_ACRN@"

# Enable agent tracing.
#
# If enabled, the default trace mode is "dynamic" and the
# default trace type is "isolated". The trace mode and type are set
# explicity with the `trace_type=` and `trace_mode=` options.
# If enabled, the agent will generate OpenTelemetry trace spans.
#
# Notes:
#
# - Tracing is ONLY enabled when `enable_tracing` is set: explicitly
# setting `trace_mode=` and/or `trace_type=` without setting `enable_tracing`
# will NOT activate agent tracing.
#
# - See https://github.com/kata-containers/agent/blob/master/TRACING.md for
# full details.
# - If the runtime also has tracing enabled, the agent spans will be
# associated with the appropriate runtime parent span.
# - If enabled, the runtime will wait for the container to shutdown,
# increasing the container shutdown time slightly.
#
# (default: disabled)
#enable_tracing = true
#
#trace_mode = "dynamic"
#trace_type = "isolated"

# Enable debug console.

Expand Down
Loading

0 comments on commit 07104ec

Please sign in to comment.