From 260e1f89297f650d5daa9e8a698671ef62b625bb Mon Sep 17 00:00:00 2001 From: Sam Whittle Date: Fri, 19 Jun 2026 10:28:23 +0200 Subject: [PATCH] [Dataflow Java] Fix incorrect warning log when specifying logger override. --- .../DataflowWorkerLoggingInitializer.java | 12 +++-- .../DataflowWorkerLoggingInitializerTest.java | 44 ++++++++++++++++++- 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/DataflowWorkerLoggingInitializer.java b/runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/DataflowWorkerLoggingInitializer.java index 0f1b5c2750bc..d854ae74ebaf 100644 --- a/runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/DataflowWorkerLoggingInitializer.java +++ b/runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/logging/DataflowWorkerLoggingInitializer.java @@ -217,17 +217,16 @@ private static void applyOverridesToLogger( boolean directLoggingEnabled) { if (!directLoggingEnabled) { logger.setLevel(overrides.disk); - if (overrides.disk.intValue() != Level.OFF.intValue()) { + if (overrides.direct.intValue() != Level.OFF.intValue()) { LOG.warn( - "Ignoring the disk logging level override for {} because --defaultWorkerDirectLoggerLevel was OFF.", + "Ignoring the direct logging level override for {} because --defaultWorkerDirectLoggerLevel was OFF.", logger.getName()); } return; } // Configure the logger to accept logs of the lower value. The log will be sent directly based - // upon the default - // nonDirectLogLevel or the provided hint. + // upon the default nonDirectLogLevel or the provided hint. if (overrides.direct.intValue() < overrides.disk.intValue()) { logger.setLevel(overrides.direct); if (overrides.disk.intValue() != defaultNonDirectLogLevel.intValue()) { @@ -245,9 +244,8 @@ private static void applyOverridesToLogger( logger.setLevel(overrides.disk); if (overrides.disk.intValue() < defaultNonDirectLogLevel.intValue()) { // We need to provide the hint as otherwise the default would be used and send the log - // directly which is not - // desired. We don't bother if the threshold was increased as the right decision would still - // be made. + // directly which is not desired. We don't bother if the threshold was increased as the + // right decision would still be made. logger.setResourceBundle( DataflowWorkerLoggingHandler.resourceBundleForNonDirectLogLevelHint(overrides.disk)); } diff --git a/runners/google-cloud-dataflow-java/worker/src/test/java/org/apache/beam/runners/dataflow/worker/logging/DataflowWorkerLoggingInitializerTest.java b/runners/google-cloud-dataflow-java/worker/src/test/java/org/apache/beam/runners/dataflow/worker/logging/DataflowWorkerLoggingInitializerTest.java index 7537c17a17bb..7569590b63f0 100644 --- a/runners/google-cloud-dataflow-java/worker/src/test/java/org/apache/beam/runners/dataflow/worker/logging/DataflowWorkerLoggingInitializerTest.java +++ b/runners/google-cloud-dataflow-java/worker/src/test/java/org/apache/beam/runners/dataflow/worker/logging/DataflowWorkerLoggingInitializerTest.java @@ -134,7 +134,7 @@ public void testWithSdkHarnessConfigurationOverride() { } @Test - public void testWithWorkerCustomLogLevels() { + public void testWithWorkerCustomLogLevels() throws IOException { DataflowWorkerLoggingOptions options = PipelineOptionsFactory.as(DataflowWorkerLoggingOptions.class); options.setWorkerLogLevelOverrides( @@ -153,6 +153,9 @@ public void testWithWorkerCustomLogLevels() { assertEquals(Level.SEVERE, bLogger.getLevel()); assertEquals(0, bLogger.getHandlers().length); assertTrue(aLogger.getUseParentHandlers()); + + List logLines = retrieveLogLines(); + assertThat(logLines, not(hasItem(containsString("Ignoring the direct logging level")))); } @Test @@ -176,7 +179,7 @@ public void testWithDirectLogging() { } @Test - public void testWithSdkHarnessCustomLogLevels() { + public void testWithSdkHarnessCustomLogLevels() throws IOException { SdkHarnessOptions options = PipelineOptionsFactory.as(SdkHarnessOptions.class); options.setSdkHarnessLogLevelOverrides( new SdkHarnessLogLevelOverrides() @@ -194,6 +197,43 @@ public void testWithSdkHarnessCustomLogLevels() { assertEquals(Level.SEVERE, bLogger.getLevel()); assertEquals(0, bLogger.getHandlers().length); assertTrue(aLogger.getUseParentHandlers()); + + List logLines = retrieveLogLines(); + assertThat(logLines, not(hasItem(containsString("Ignoring the direct logging level")))); + } + + @Test + public void testWithSdkHarnessCustomLogLevels_DirectSpecifiedNotEnabled() throws IOException { + SdkHarnessOptions options = PipelineOptionsFactory.as(SdkHarnessOptions.class); + options.setSdkHarnessLogLevelOverrides( + new SdkHarnessLogLevelOverrides() + .addOverrideForName("B", SdkHarnessOptions.LogLevel.DEBUG) + .addOverrideForName("C", SdkHarnessOptions.LogLevel.ERROR)); + DataflowWorkerLoggingOptions loggingOptions = options.as(DataflowWorkerLoggingOptions.class); + loggingOptions.setWorkerDirectLogLevelOverrides( + new WorkerLogLevelOverrides() + .addOverrideForName("A", DataflowWorkerLoggingOptions.Level.TRACE) + .addOverrideForName("C", DataflowWorkerLoggingOptions.Level.TRACE)); + + DataflowWorkerLoggingInitializer.configure(options.as(DataflowWorkerLoggingOptions.class)); + + Logger aLogger = LogManager.getLogManager().getLogger("A"); + assertEquals(0, aLogger.getHandlers().length); + assertEquals(Level.INFO, aLogger.getLevel()); + assertTrue(aLogger.getUseParentHandlers()); + + Logger bLogger = LogManager.getLogManager().getLogger("B"); + assertEquals(Level.FINE, bLogger.getLevel()); + assertEquals(0, bLogger.getHandlers().length); + assertTrue(aLogger.getUseParentHandlers()); + + Logger cLogger = LogManager.getLogManager().getLogger("C"); + assertEquals(Level.SEVERE, cLogger.getLevel()); + assertEquals(0, cLogger.getHandlers().length); + assertTrue(aLogger.getUseParentHandlers()); + + List logLines = retrieveLogLines(); + assertThat(logLines, hasItem(containsString("Ignoring the direct logging level"))); } @Test