Merge pull request #1097 from jvanz/fix-otel

fix(test): concurrent access to global OTEL configuration.

Author: flavio

.github/workflows/ci.yml

@@ -19,9 +19,8 @@ jobs:
           profile: minimal
           toolchain: stable
           override: true
-      - uses: actions-rs/cargo@844f36862e911db73fe0815f00a4a2602c279505 # v1.0.3
-        with:
-          command: check
+      - name: Run cargo check
+        run: cargo check
 
   version-check:
     name: Check Cargo.toml version
@@ -50,10 +49,8 @@ jobs:
           profile: minimal
           toolchain: stable
           override: true
-      - uses: actions-rs/cargo@844f36862e911db73fe0815f00a4a2602c279505 # v1.0.3
-        with:
-          command: test
-          args: --workspace --lib
+      - name: Run unit-tests coverage
+        run: make unit-tests
 
   integration-tests:
     name: Integration tests
@@ -65,10 +62,8 @@ jobs:
           profile: minimal
           toolchain: stable
           override: true
-      - uses: actions-rs/cargo@844f36862e911db73fe0815f00a4a2602c279505 # v1.0.3
-        with:
-          command: test
-          args: --test '*'
+      - name: Run integration-tests coverage
+        run: make integration-tests
 
   fmt:
     name: Rustfmt
@@ -80,11 +75,8 @@ jobs:
           profile: minimal
           toolchain: stable
           override: true
-      - run: rustup component add rustfmt
-      - uses: actions-rs/cargo@844f36862e911db73fe0815f00a4a2602c279505 # v1.0.3
-        with:
-          command: fmt
-          args: --all -- --check
+      - name: Run Rust format check
+        run: make fmt
 
   clippy:
     name: Clippy
@@ -96,11 +88,8 @@ jobs:
           profile: minimal
           toolchain: stable
           override: true
-      - run: rustup component add clippy
-      - uses: actions-rs/cargo@844f36862e911db73fe0815f00a4a2602c279505 # v1.0.3
-        with:
-          command: clippy
-          args: -- -D warnings
+      - name: Run Clippy
+        run: make lint
 
   coverage:
     name: coverage
@@ -135,6 +124,15 @@ jobs:
           directory: coverage/integration-tests
           flags: integration-tests
           verbose: true
+      - name: Upload OTEL integration-tests coverage to Codecov
+        uses: codecov/codecov-action@13ce06bfc6bbe3ecf90edbbf1bc32fe5978ca1d3 # v5.3.1
+        env:
+          CODECOV_TOKEN: ${{ secrets.CODECOV_ORG_TOKEN }}
+        with:
+          name: otel-integration-tests
+          directory: coverage/otel-integration-tests
+          flags: integration-tests
+          verbose: true
 
   docs:
     name: Update documentation

Cargo.toml

@@ -91,3 +91,6 @@ reqwest = { version = "0.12", default-features = false, features = [
   "http2",
   "rustls-tls-manual-roots",
 ] }
+
+[features]
+otel_tests = []

Makefile

@@ -32,6 +32,7 @@ unit-tests: fmt lint
 .PHONY: integration-test
 integration-tests: fmt lint
 	cargo test --test '*'
+	cargo test --features otel_tests -- test_otel
 
 .PHONY: coverage
 coverage: coverage-unit-tests coverage-integration-tests
@@ -45,8 +46,11 @@ coverage-unit-tests:
 .PHONY: coverage-integration-tests
 coverage-integration-tests:
 	cargo tarpaulin --verbose --skip-clean --engine=llvm \
-		--all-features --implicit-test-threads --test integration_test \
+		--implicit-test-threads --test integration_test \
 		--out xml --out html --output-dir coverage/integration-tests
+	cargo tarpaulin --verbose --skip-clean --engine=llvm \
+		--features otel_tests --implicit-test-threads --test integration_test \
+		--out xml --out html --output-dir coverage/otel-integration-tests -- test_otel
 
 .PHONY: clean
 clean:

tests/integration_test.rs

@@ -1,14 +1,12 @@
 mod common;
 
-use std::io::BufReader;
+use std::path::PathBuf;
 use std::{
     collections::{BTreeSet, HashMap},
-    fs::{set_permissions, File, Permissions},
-    io::BufRead,
-    os::unix::fs::PermissionsExt,
-    path::PathBuf,
     time::Duration,
 };
+#[cfg(feature = "otel_tests")]
+use std::{fs::File, io::BufRead};
 
 use common::{app, setup};
 
@@ -26,18 +24,9 @@ use policy_evaluator::{
 use policy_server::{
     api::admission_review::AdmissionReviewResponse,
     config::{PolicyMode, PolicyOrPolicyGroup},
-    metrics::setup_metrics,
-    tracing::setup_tracing,
 };
-use rcgen::{BasicConstraints, CertificateParams, DnType, IsCa, KeyPair};
 use regex::Regex;
 use rstest::*;
-use tempfile::NamedTempFile;
-use testcontainers::{
-    core::{Mount, WaitFor},
-    runners::AsyncRunner,
-    GenericImage, ImageExt,
-};
 use tokio::fs;
 use tower::ServiceExt;
 
@@ -804,10 +793,23 @@ async fn test_detect_certificate_rotation() {
     }
 }
 
-// This test is flaky, needs fix. Ignoring for release.
-#[ignore]
+// The OTEL test is behind a feature flag because it needs to ensure that the
+// global OTEL configuration is not overwritten by other concurrent tests.
 #[tokio::test]
+#[cfg(feature = "otel_tests")]
 async fn test_otel() {
+    use policy_server::{metrics::setup_metrics, tracing::setup_tracing};
+    use std::{
+        fs::{set_permissions, Permissions},
+        os::unix::fs::PermissionsExt,
+    };
+    use tempfile::NamedTempFile;
+    use testcontainers::{
+        core::{Mount, WaitFor},
+        runners::AsyncRunner,
+        GenericImage, ImageExt,
+    };
+
     setup();
 
     let otelc_config_path =
@@ -970,9 +972,12 @@ async fn test_otel() {
     otelc.stop().await.unwrap();
 }
 
+#[cfg(feature = "otel_tests")]
 async fn parse_exporter_output(
     exporter_output_file: &File,
 ) -> serde_json::Result<serde_json::Value> {
+    use std::io::BufReader;
+
     let mut reader = BufReader::new(exporter_output_file);
 
     // read only the first entry in the output file
@@ -984,7 +989,10 @@ async fn parse_exporter_output(
     serde_json::from_str(&exporter_output)
 }
 
+#[cfg(feature = "otel_tests")]
 fn generate_tls_certs() -> (String, String, String) {
+    use rcgen::{BasicConstraints, CertificateParams, DnType, IsCa, KeyPair};
+
     let ca_key = KeyPair::generate().unwrap();
     let mut params = CertificateParams::new(vec!["My Test CA".to_string()]).unwrap();
     params.is_ca = IsCa::Ca(BasicConstraints::Unconstrained);