Skip to content

Commit

Permalink
Remove step-trace and trace features
Browse files Browse the repository at this point in the history
Those were used to generate steps graph at runtime, but we've migrated to the new step engine that generates it at compile time, so we no longer need a script and this set of features
  • Loading branch information
akoshelev committed Sep 2, 2024
1 parent 77ac0b0 commit 42e01b7
Show file tree
Hide file tree
Showing 9 changed files with 16 additions and 226 deletions.
1 change: 0 additions & 1 deletion ipa-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
24 changes: 5 additions & 19 deletions ipa-core/benches/oneshot/ipa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 0 additions & 2 deletions ipa-core/src/telemetry/stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
Expand Down
10 changes: 4 additions & 6 deletions ipa-core/src/telemetry/step_stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
11 changes: 5 additions & 6 deletions ipa-core/src/test_fixture/logging.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::sync::Once;

use tracing_subscriber::fmt::format::FmtSpan;

/// Set up logging for IPA
///
/// ## Panics
Expand Down Expand Up @@ -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(
Expand Down
3 changes: 1 addition & 2 deletions ipa-core/src/test_fixture/world.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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();
Expand Down
2 changes: 0 additions & 2 deletions ipa-step/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
11 changes: 1 addition & 10 deletions ipa-step/src/descriptive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
Expand Down
178 changes: 0 additions & 178 deletions scripts/collect_steps.py

This file was deleted.

0 comments on commit 42e01b7

Please sign in to comment.