From a4a673b780af257018db6d0019af0f454dbe7f5b Mon Sep 17 00:00:00 2001 From: Akihito Nakano Date: Thu, 3 Oct 2024 22:53:36 +0900 Subject: [PATCH] Output network-test logs into files in CI (#6355) * Add ci_logger * Update artifact name * Add env var * Add fork_name * Fix clippy error * Add comments --- .github/workflows/test-suite.yml | 11 ++++ beacon_node/beacon_chain/src/test_utils.rs | 62 ++++++++++++++++--- beacon_node/network/Cargo.toml | 1 + .../network/src/sync/block_lookups/tests.rs | 12 +++- 4 files changed, 74 insertions(+), 12 deletions(-) diff --git a/.github/workflows/test-suite.yml b/.github/workflows/test-suite.yml index 769b889de4d..aff9a71b4ad 100644 --- a/.github/workflows/test-suite.yml +++ b/.github/workflows/test-suite.yml @@ -173,8 +173,19 @@ jobs: channel: stable cache-target: release bins: cargo-nextest + - name: Create CI logger dir + run: mkdir ${{ runner.temp }}/network_test_logs - name: Run network tests for all known forks run: make test-network + env: + TEST_FEATURES: portable,ci_logger + CI_LOGGER_DIR: ${{ runner.temp }}/network_test_logs + - name: Upload logs + uses: actions/upload-artifact@v4 + with: + name: network_test_logs + path: ${{ runner.temp }}/network_test_logs + slasher-tests: name: slasher-tests needs: [check-labels] diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index ce36c8ca216..344820c6a24 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -43,13 +43,15 @@ use rayon::prelude::*; use sensitive_url::SensitiveUrl; use slog::{o, Drain, Logger}; use slog_async::Async; -use slog_term::{FullFormat, TermDecorator}; +use slog_term::{FullFormat, PlainSyncDecorator, TermDecorator}; use slot_clock::{SlotClock, TestingSlotClock}; use state_processing::per_block_processing::compute_timestamp_at_slot; use state_processing::state_advance::complete_state_advance; use std::borrow::Cow; use std::collections::{HashMap, HashSet}; use std::fmt; +use std::fs::{File, OpenOptions}; +use std::io::BufWriter; use std::str::FromStr; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::{Arc, LazyLock}; @@ -68,6 +70,8 @@ use types::{typenum::U4294967296, *}; pub const HARNESS_GENESIS_TIME: u64 = 1_567_552_690; // Environment variable to read if `fork_from_env` feature is enabled. pub const FORK_NAME_ENV_VAR: &str = "FORK_NAME"; +// Environment variable to read if `ci_logger` feature is enabled. +pub const CI_LOGGER_DIR_ENV_VAR: &str = "CI_LOGGER_DIR"; // Default target aggregators to set during testing, this ensures an aggregator at each slot. // @@ -2750,15 +2754,55 @@ pub struct MakeAttestationOptions { pub fork: Fork, } -pub fn build_log(level: slog::Level, enabled: bool) -> Logger { - let decorator = TermDecorator::new().build(); - let drain = FullFormat::new(decorator).build().fuse(); - let drain = Async::new(drain).build().fuse(); +pub enum LoggerType { + Test, + // The logs are output to files for each test. + CI, + // No logs will be printed. + Null, +} - if enabled { - Logger::root(drain.filter_level(level).fuse(), o!()) - } else { - Logger::root(drain.filter(|_| false).fuse(), o!()) +fn ci_decorator() -> PlainSyncDecorator> { + let log_dir = std::env::var(CI_LOGGER_DIR_ENV_VAR).unwrap_or_else(|e| { + panic!("{CI_LOGGER_DIR_ENV_VAR} env var must be defined when using ci_logger: {e:?}"); + }); + let fork_name = std::env::var(FORK_NAME_ENV_VAR) + .map(|s| format!("{s}_")) + .unwrap_or_default(); + // The current test name can be got via the thread name. + let test_name = std::thread::current() + .name() + .unwrap() + .to_string() + // Colons are not allowed in files that are uploaded to GitHub Artifacts. + .replace("::", "_"); + let log_path = format!("/{log_dir}/{fork_name}{test_name}.log"); + let file = OpenOptions::new() + .create(true) + .append(true) + .open(log_path) + .unwrap(); + let file = BufWriter::new(file); + PlainSyncDecorator::new(file) +} + +pub fn build_log(level: slog::Level, logger_type: LoggerType) -> Logger { + match logger_type { + LoggerType::Test => { + let drain = FullFormat::new(TermDecorator::new().build()).build().fuse(); + let drain = Async::new(drain).build().fuse(); + Logger::root(drain.filter_level(level).fuse(), o!()) + } + LoggerType::CI => { + let drain = FullFormat::new(ci_decorator()).build().fuse(); + let drain = Async::new(drain).build().fuse(); + Logger::root(drain.filter_level(level).fuse(), o!()) + } + LoggerType::Null => { + let drain = FullFormat::new(TermDecorator::new().build()).build().fuse(); + let drain = Async::new(drain).build().fuse(); + Logger::root(drain.filter(|_| false).fuse(), o!()) + } } } diff --git a/beacon_node/network/Cargo.toml b/beacon_node/network/Cargo.toml index fed346127f0..4df17617326 100644 --- a/beacon_node/network/Cargo.toml +++ b/beacon_node/network/Cargo.toml @@ -57,3 +57,4 @@ disable-backfill = [] fork_from_env = ["beacon_chain/fork_from_env"] portable = ["beacon_chain/portable"] test_logger = [] +ci_logger = [] diff --git a/beacon_node/network/src/sync/block_lookups/tests.rs b/beacon_node/network/src/sync/block_lookups/tests.rs index ffbdd43b5f1..4c73e8f8d03 100644 --- a/beacon_node/network/src/sync/block_lookups/tests.rs +++ b/beacon_node/network/src/sync/block_lookups/tests.rs @@ -15,7 +15,7 @@ use beacon_chain::data_availability_checker::Availability; use beacon_chain::eth1_chain::CachingEth1Backend; use beacon_chain::test_utils::{ build_log, generate_rand_block_and_blobs, generate_rand_block_and_data_columns, test_spec, - BeaconChainHarness, EphemeralHarnessType, NumBlobs, + BeaconChainHarness, EphemeralHarnessType, LoggerType, NumBlobs, }; use beacon_chain::validator_monitor::timestamp_now; use beacon_chain::{ @@ -103,8 +103,14 @@ struct TestRigConfig { impl TestRig { fn test_setup_with_config(config: Option) -> Self { - let enable_log = cfg!(feature = "test_logger"); - let log = build_log(slog::Level::Trace, enable_log); + let logger_type = if cfg!(feature = "test_logger") { + LoggerType::Test + } else if cfg!(feature = "ci_logger") { + LoggerType::CI + } else { + LoggerType::Null + }; + let log = build_log(slog::Level::Trace, logger_type); // Use `fork_from_env` logic to set correct fork epochs let mut spec = test_spec::();