diff --git a/etl-service/src/main/java/com/xtrmetl/etl/connector/TargetConnectorDispatcher.java b/etl-service/src/main/java/com/xtrmetl/etl/connector/TargetConnectorDispatcher.java index 72ddada9..29186cf0 100644 --- a/etl-service/src/main/java/com/xtrmetl/etl/connector/TargetConnectorDispatcher.java +++ b/etl-service/src/main/java/com/xtrmetl/etl/connector/TargetConnectorDispatcher.java @@ -186,11 +186,7 @@ private void ensureOpen( connector.close(); } catch (RuntimeException cleanupFailure) { openFailure.addSuppressed(cleanupFailure); - log.warn( - "Failed to clean up target connector after open failure id={}", - connectorId, - cleanupFailure - ); + log.warn("Failed to clean up target connector after open failure id={}", connectorId); } throw openFailure; } @@ -232,7 +228,7 @@ void closeOpenedConnectors() { connector.close(); log.info("Closed target connector id={}", connectorId); } catch (RuntimeException exception) { - log.error("Failed to close target connector id={}", connectorId, exception); + log.error("Failed to close target connector id={}", connectorId); } } } diff --git a/etl-service/src/test/java/com/xtrmetl/etl/connector/TargetConnectorDispatcherLoggingTest.java b/etl-service/src/test/java/com/xtrmetl/etl/connector/TargetConnectorDispatcherLoggingTest.java new file mode 100644 index 00000000..884e999d --- /dev/null +++ b/etl-service/src/test/java/com/xtrmetl/etl/connector/TargetConnectorDispatcherLoggingTest.java @@ -0,0 +1,161 @@ +package com.xtrmetl.etl.connector; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.boot.test.system.CapturedOutput; +import org.springframework.boot.test.system.OutputCaptureExtension; + +import java.util.List; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Verifies that target-connector lifecycle logs retain finite outcome classification without + * serializing third-party exception diagnostics. + */ +@ExtendWith(OutputCaptureExtension.class) +class TargetConnectorDispatcherLoggingTest { + + @Test + void failedOpenCleanupDoesNotLogProviderDiagnostics(CapturedOutput output) { + String openDiagnostic = "https://warehouse.example/sql?diagnostic=private-marker-8472"; + String cleanupDiagnostic = "cleanup-private-marker-8472"; + RuntimeException openFailure = new IllegalStateException(openDiagnostic); + RuntimeException cleanupFailure = new IllegalArgumentException(cleanupDiagnostic); + FailingLifecycleConnector connector = new FailingLifecycleConnector(openFailure, cleanupFailure, false); + TargetConnectorDispatcher dispatcher = dispatcher(connector); + + RuntimeException thrown = assertThrows( + RuntimeException.class, + () -> dispatcher.dispatch("databricks", List.of()) + ); + + assertSame(openFailure, thrown); + assertEquals(1, thrown.getSuppressed().length); + assertSame(cleanupFailure, thrown.getSuppressed()[0]); + assertSafeLogs( + output, + "Failed to clean up target connector after open failure id=databricks", + cleanupDiagnostic, + "IllegalArgumentException", + "TargetConnectorDispatcherLoggingTest" + ); + } + + @Test + void shutdownCloseFailureDoesNotLogProviderDiagnosticsAndRemainsBestEffort(CapturedOutput output) { + String closeDiagnostic = "jdbc:vendor://internal.example/prod?diagnostic=private-marker-8472"; + FailingLifecycleConnector connector = new FailingLifecycleConnector( + null, + new IllegalStateException(closeDiagnostic), + true + ); + TargetConnectorDispatcher dispatcher = dispatcher(connector); + + dispatcher.dispatch("databricks", List.of()); + dispatcher.closeOpenedConnectors(); + + assertEquals(List.of("open", "write", "close"), connector.events); + assertSafeLogs( + output, + "Failed to close target connector id=databricks", + closeDiagnostic, + "IllegalStateException", + "TargetConnectorDispatcherLoggingTest" + ); + assertFalse(Boolean.TRUE.equals(dispatcher.catalog().stream() + .filter(row -> "databricks".equals(row.get("id"))) + .findFirst() + .orElseThrow() + .get("opened"))); + } + + private static TargetConnectorDispatcher dispatcher(TargetConnector connector) { + TargetConnectorRegistry registry = new TargetConnectorRegistry(); + registry.register(connector); + return new TargetConnectorDispatcher(registry, enabledDatabricksProperties()); + } + + private static ConnectorProperties enabledDatabricksProperties() { + ConnectorProperties properties = new ConnectorProperties(); + properties.getDatabricks().setEnabled(true); + properties.getDatabricks().setHost("host"); + properties.getDatabricks().setHttpPath("/sql"); + properties.getDatabricks().setToken("test-value"); + properties.getDatabricks().setCatalog("catalog"); + properties.getDatabricks().setSchema("schema"); + properties.getDatabricks().setTable("table_name"); + return properties; + } + + private static void assertSafeLogs(CapturedOutput output, String expected, String... forbidden) { + String logs = output.getOut() + output.getErr(); + assertTrue(logs.contains(expected)); + for (String value : forbidden) { + assertFalse(logs.contains(value), () -> "Log output exposed forbidden value: " + value); + } + } + + private static final class FailingLifecycleConnector implements TargetConnector { + private final RuntimeException openFailure; + private final RuntimeException closeFailure; + private final boolean openSucceeds; + private final java.util.ArrayList events = new java.util.ArrayList<>(); + + private FailingLifecycleConnector( + RuntimeException openFailure, + RuntimeException closeFailure, + boolean openSucceeds + ) { + this.openFailure = openFailure; + this.closeFailure = closeFailure; + this.openSucceeds = openSucceeds; + } + + @Override + public String id() { + return "databricks"; + } + + @Override + public String displayName() { + return "Failing lifecycle connector"; + } + + @Override + public ConnectorStatus status() { + return ConnectorStatus.SUPPORTED; + } + + @Override + public void validate(Map config) { + // This fake intentionally accepts the validated dispatcher fixture configuration. + } + + @Override + public void open(Map config) { + events.add("open"); + if (!openSucceeds) { + throw openFailure; + } + } + + @Override + public void write(List batch) { + events.add("write"); + } + + @Override + public void close() { + events.add("close"); + if (closeFailure != null) { + throw closeFailure; + } + } + } +} diff --git a/etl-service/src/test/java/com/xtrmetl/etl/connector/TargetConnectorLoggingPolicyTest.java b/etl-service/src/test/java/com/xtrmetl/etl/connector/TargetConnectorLoggingPolicyTest.java new file mode 100644 index 00000000..4891aee3 --- /dev/null +++ b/etl-service/src/test/java/com/xtrmetl/etl/connector/TargetConnectorLoggingPolicyTest.java @@ -0,0 +1,27 @@ +package com.xtrmetl.etl.connector; + +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.junit.jupiter.api.Assertions.assertFalse; + +/** + * Guards the target-connector lifecycle logging boundary against publishing provider exceptions. + */ +class TargetConnectorLoggingPolicyTest { + + @Test + void lifecycleFailureLogsDoNotCarryExceptionObjects() throws IOException { + String source = Files.readString( + Path.of("src/main/java/com/xtrmetl/etl/connector/TargetConnectorDispatcher.java"), + StandardCharsets.UTF_8 + ); + + assertFalse(source.contains("connectorId,\n cleanupFailure")); + assertFalse(source.contains("connectorId, exception")); + } +}