Merge pull request #1248 from akoshelev/rem-trace

Remove step-trace and trace features

ipa-core/Cargo.toml

@@ -60,7 +60,6 @@ real-world-infra = []
 dhat-heap = ["cli", "test-fixture"]
 # Enable this feature to enable our colossally weak Fp31.
 weak-field = []
-step-trace = ["ipa-step/trace"]
 # Enable using more than one thread for protocol execution. Most of the parallelism occurs at parallel/seq_join operations
 multi-threading = ["async-scoped"]
 # Enable tokio task profiling. Requires tokio_unstable flag to be passed to the compiler.

ipa-core/benches/oneshot/ipa.rs

@@ -1,6 +1,6 @@
 use std::{
  env,
- num::{NonZeroU32, NonZeroU64, NonZeroUsize},
+ num::{NonZeroU32, NonZeroUsize},
  time::Instant,
 };
 
@@ -122,25 +122,11 @@ async fn run(args: Args) -> Result<(), Error> {
  q = args.query_size
  );
  let rng = StdRng::seed_from_u64(seed);
- let event_gen_config = if cfg!(feature = "step-trace") {
- // For the steps collection, compact gate requires:
- // * At least one user with the same number of dynamic steps as defined for `UserNthRowStep::Row`.
- // * Enough records to exercise the saturating addition case in aggregation.
- EventGeneratorConfig {
- user_count: NonZeroU64::new(5).unwrap(),
- max_trigger_value: NonZeroU32::try_from(args.max_trigger_value).unwrap(),
- max_breakdown_key: NonZeroU32::try_from(args.breakdown_keys).unwrap(),
- min_events_per_user: NonZeroU32::new(64).unwrap(),
- max_events_per_user: NonZeroU32::new(64).unwrap(),
- ..Default::default()
- }
- } else {
- EventGeneratorConfig {
- max_trigger_value: NonZeroU32::try_from(args.max_trigger_value).unwrap(),
- max_breakdown_key: NonZeroU32::try_from(args.breakdown_keys).unwrap(),
- max_events_per_user: NonZeroU32::try_from(args.records_per_user).unwrap(),
- ..Default::default()
- }
+ let event_gen_config = EventGeneratorConfig {
+ max_trigger_value: NonZeroU32::try_from(args.max_trigger_value).unwrap(),
+ max_breakdown_key: NonZeroU32::try_from(args.breakdown_keys).unwrap(),
+ max_events_per_user: NonZeroU32::try_from(args.records_per_user).unwrap(),
+ ..Default::default()
  };
  let raw_data = EventGenerator::with_config(rng, event_gen_config)
  .take(args.query_size)

ipa-core/src/telemetry/stats.rs

@@ -35,7 +35,6 @@ pub struct CounterDetails {
 pub struct Metrics {
  pub counters: HashMap<KeyName, CounterDetails>,
  pub metric_description: HashMap<KeyName, SharedString>,
- pub print_header: bool,
 }
 
 impl CounterDetails {
@@ -81,7 +80,6 @@ impl Metrics {
  let mut this = Metrics {
  counters: HashMap::new(),
  metric_description: HashMap::new(),
- print_header: !cfg!(feature = "step-trace"),
  };
 
  let snapshot = snapshot.into_vec();

ipa-core/src/telemetry/step_stats.rs

@@ -38,12 +38,10 @@ impl CsvExporter for Metrics {
  // then dump them to the provided Write interface
  // TODO: include role dimension. That requires rethinking `Metrics` implementation
  // because it does not allow such breakdown atm.
- if self.print_header {
- writeln!(
- w,
- "Step,Records sent,Bytes sent,Indexed PRSS,Sequential PRSS,Step narrowed"
- )?;
- }
+ writeln!(
+ w,
+ "Step,Records sent,Bytes sent,Indexed PRSS,Sequential PRSS,Step narrowed"
+ )?;
  for (step, stats) in steps_stats.all_steps() {
  writeln!(
  w,

ipa-core/src/test_fixture/logging.rs

@@ -1,5 +1,7 @@
 use std::sync::Once;
 
+use tracing_subscriber::fmt::format::FmtSpan;
+
 /// Set up logging for IPA
 ///
 /// ## Panics
@@ -30,12 +32,9 @@ pub fn setup() {
  Level::INFO.into()
  };
 
- let fmt_layer = fmt::layer().with_test_writer();
- #[cfg(not(feature = "step-trace"))]
- let fmt_layer = {
- use tracing_subscriber::fmt::format::FmtSpan;
- fmt_layer.with_span_events(FmtSpan::NEW | FmtSpan::CLOSE)
- };
+ let fmt_layer = fmt::layer()
+ .with_test_writer()
+ .with_span_events(FmtSpan::NEW | FmtSpan::CLOSE);
 
  tracing_subscriber::registry()
  .with(

ipa-core/src/test_fixture/world.rs

@@ -250,7 +250,7 @@ impl TestWorld<NotSharded> {
 
 impl<S: ShardingScheme> Drop for TestWorld<S> {
  fn drop(&mut self) {
- if tracing::span_enabled!(Level::DEBUG) || cfg!(feature = "step-trace") {
+ if tracing::span_enabled!(Level::DEBUG) {
  let metrics = self.metrics_handle.snapshot();
  metrics.export(&mut stdout()).unwrap();
  }
@@ -266,7 +266,6 @@ impl<S: ShardingScheme> TestWorld<S> {
  pub fn with_config(config: &TestWorldConfig) -> Self {
  logging::setup();
  // Print to stdout so that it appears in test runs only on failure.
- // scripts/collect_steps.py must be updated if the message text changes.
  println!("TestWorld random seed {seed}", seed = config.seed);
 
  let shard_count = ShardIndex::try_from(S::SHARDS).unwrap();

ipa-step/Cargo.toml

@@ -8,10 +8,8 @@ edition = "2021"
 build = ["name", "prettyplease", "proc-macro2", "quote", "syn"]
 name = []
 string-step = []
-trace = ["metrics"]
 
 [dependencies]
-metrics = { version = "0.21.0", optional = true }
 prettyplease = { version = "0.2", optional = true }
 proc-macro2 = { version = "1", optional = true }
 quote = { version = "1.0.36", optional = true }

ipa-step/src/descriptive.rs

@@ -87,16 +87,7 @@ impl<S: Step + ?Sized> StepNarrow<S> for Descriptive {
  assert!(!s.contains('/'), "The string for a step cannot contain '/'");
  }
 
- let mut id = self.id.clone() + "/";
- #[cfg(feature = "trace")]
- {
- id += [std::any::type_name::<S>(), "::"].concat().as_ref();
- }
- id += step.as_ref();
- #[cfg(feature = "trace")]
- {
- metrics::increment_counter!(STEP_NARROWED, STEP => id.clone());
- }
+ let id = format!("{}/{}", self.id, step.as_ref());
 
  Self { id }
  }