fix(agent-data-plane): ensure logs are flushed before exiting (#1201)

Author: tobz

.gitlab/e2e.yml

@@ -115,11 +115,17 @@ test-integration:
   retry: 2
   timeout: 10m
   image: "${SALUKI_BUILD_CI_IMAGE}"
+  artifacts:
+    expire_in: 1 week
+    paths:
+      - integration-logs/
+    when: always
   variables:
     # Looks weird but we need to set this so that the test runner knows the right image path to pull the `alpine` image, which is required by
     # `airlock` to properly configure some permissions on the shared volume before the target container is started. We don't have access to
     # Docker Hub so the default of `alpine:latest` is insufficient.
     GROUND_TRUTH_ALPINE_IMAGE: registry.ddbuild.io/alpine:latest
+    PANORAMIC_LOG_DIR: integration-logs
   script:
     # Copy the bundled Agent/ADP image we built to the local Docker instance so we can share the integration
     # test configurations between CI and local test runs.

Makefile

@@ -557,13 +557,13 @@ build-panoramic: ## Builds the panoramic binary (ADP integration test runner)
 test-integration: build-panoramic build-datadog-agent-image
 test-integration: ## Runs all ADP integration tests
 	@echo "[*] Running ADP integration tests..."
-	@target/release/panoramic run -d $(shell pwd)/test/integration/cases
+	@target/release/panoramic run -d $(shell pwd)/test/integration/cases $(if $(PANORAMIC_LOG_DIR),-l $(PANORAMIC_LOG_DIR))
 
 .PHONY: test-integration-quick
 test-integration-quick: build-panoramic
 test-integration-quick: ## Runs ADP integration tests (assumes images already built)
 	@echo "[*] Running ADP integration tests (quick mode)..."
-	@target/release/panoramic run -d $(shell pwd)/test/integration/cases
+	@target/release/panoramic run -d $(shell pwd)/test/integration/cases $(if $(PANORAMIC_LOG_DIR),-l $(PANORAMIC_LOG_DIR))
 
 .PHONY: list-integration-tests
 list-integration-tests: build-panoramic

bin/agent-data-plane/src/main.rs

@@ -5,10 +5,10 @@
 
 #![deny(warnings)]
 #![deny(missing_docs)]
-use std::time::Instant;
+use std::{path::PathBuf, time::Instant};
 
 use saluki_app::bootstrap::AppBootstrapper;
-use saluki_config::ConfigurationLoader;
+use saluki_config::{ConfigurationLoader, GenericConfiguration};
 use saluki_error::{ErrorContext as _, GenericError};
 use tracing::{error, info, warn};
 
@@ -64,25 +64,41 @@ async fn main() -> Result<(), GenericError> {
         .error_context("Failed to complete bootstrap phase.")?;
 
     // Run the given subcommand.
-    match cli.action {
+    let maybe_exit_code = run_inner(cli.action, started, bootstrap_config_path, bootstrap_config).await?;
+
+    // Drop the bootstrap guard to ensure logs are flushed, etc.
+    drop(_bootstrap_guard);
+
+    // Exit with the specific exit code, if one was provided.
+    if let Some(exit_code) = maybe_exit_code {
+        std::process::exit(exit_code);
+    }
+
+    Ok(())
+}
+
+async fn run_inner(
+    action: Action, started: Instant, bootstrap_config_path: PathBuf, bootstrap_config: GenericConfiguration,
+) -> Result<Option<i32>, GenericError> {
+    match action {
         Action::Run(cmd) => {
             // Populate our PID file, if configured.
             if let Some(pid_file) = &cmd.pid_file {
                 let pid = std::process::id();
                 if let Err(e) = std::fs::write(pid_file, pid.to_string()) {
                     error!(error = %e, path = %pid_file.display(), "Failed to update PID file. Exiting.");
-                    std::process::exit(1);
+                    return Ok(Some(1));
                 }
             }
 
             let exit_code = match handle_run_command(started, bootstrap_config_path, bootstrap_config).await {
                 Ok(()) => {
                     info!("Agent Data Plane stopped.");
-                    0
+                    None
                 }
                 Err(e) => {
                     error!("{:?}", e);
-                    1
+                    Some(1)
                 }
             };
 
@@ -93,12 +109,14 @@ async fn main() -> Result<(), GenericError> {
                 }
             }
 
-            std::process::exit(exit_code);
+            if let Some(exit_code) = exit_code {
+                return Ok(Some(exit_code));
+            }
         }
         Action::Debug(cmd) => handle_debug_command(&bootstrap_config, cmd).await,
         Action::Config(_) => handle_config_command(&bootstrap_config).await,
         Action::Dogstatsd(cmd) => handle_dogstatsd_command(&bootstrap_config, cmd).await,
     }
 
-    Ok(())
+    Ok(None)
 }

bin/correctness/panoramic/src/cli.rs

@@ -49,6 +49,10 @@ pub struct RunCommand {
     #[argh(switch)]
     pub no_tui: bool,
 
+    /// directory to write container logs to (default: auto-generated temp dir)
+    #[argh(option, short = 'l')]
+    pub log_dir: Option<PathBuf>,
+
     /// skip writing container logs to disk
     #[argh(switch)]
     pub no_logs: bool,

bin/correctness/panoramic/src/main.rs

@@ -74,7 +74,7 @@ fn initialize_logging() {
         .init();
 }
 
-async fn run_tests(cmd: cli::RunCommand, use_tui: bool) -> ExitCode {
+async fn run_tests(mut cmd: cli::RunCommand, use_tui: bool) -> ExitCode {
     let mut test_cases = match discover_tests(&cmd.test_dir) {
         Ok(tests) => tests,
         Err(e) => {
@@ -115,8 +115,12 @@ async fn run_tests(cmd: cli::RunCommand, use_tui: bool) -> ExitCode {
     let log_dir = if cmd.no_logs {
         None
     } else {
-        match create_log_dir() {
-            Ok(dir) => Some(dir),
+        let dir = cmd.log_dir.take().unwrap_or_else(|| {
+            let timestamp = Local::now().format("%Y%m%d-%H%M%S");
+            std::env::temp_dir().join(format!("panoramic-{}", timestamp))
+        });
+        match std::fs::create_dir_all(&dir) {
+            Ok(()) => Some(dir),
             Err(e) => {
                 if use_tui {
                     eprintln!("Failed to create log directory: {}", e);
@@ -156,15 +160,6 @@ async fn run_tests(cmd: cli::RunCommand, use_tui: bool) -> ExitCode {
     }
 }
 
-/// Create a unique temporary directory for container logs.
-fn create_log_dir() -> std::io::Result<PathBuf> {
-    let timestamp = Local::now().format("%Y%m%d-%H%M%S");
-    let dir_name = format!("panoramic-{}", timestamp);
-    let log_dir = std::env::temp_dir().join(dir_name);
-    std::fs::create_dir_all(&log_dir)?;
-    Ok(log_dir)
-}
-
 /// Run with the TUI consumer.
 async fn run_with_tui_consumer(
     rx: mpsc::UnboundedReceiver<TestEvent>, cancel_token: CancellationToken, log_dir: Option<PathBuf>,