Use NoOpDebugDataDumper when disabled in config

courtneyeh · Apr 15, 2024 · e7d6be9 · e7d6be9
1 parent 92db86a
commit e7d6be9
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 82 deletions.
diff --git a/...statetransition/src/main/java/tech/pegasys/teku/statetransition/util/DebugDataDumper.java b/...statetransition/src/main/java/tech/pegasys/teku/statetransition/util/DebugDataDumper.java
@@ -41,12 +41,9 @@ public class DebugDataDumper {
   private boolean enabled;
   private final Path directory;
 
-  public DebugDataDumper(final Path directory, final boolean enabled) {
-    this.enabled = enabled;
+  public DebugDataDumper(final Path directory) {
+    this.enabled = true;
     this.directory = directory;
-    if (!enabled) {
-      return;
-    }
 
     final Path gossipMessagesPath = this.directory.resolve(GOSSIP_MESSAGES_DIR);
     createDirectory(gossipMessagesPath, GOSSIP_MESSAGES_DIR, "gossip messages");

diff --git a/...sition/src/main/java/tech/pegasys/teku/statetransition/util/noop/NoOpDebugDataDumper.java b/...sition/src/main/java/tech/pegasys/teku/statetransition/util/noop/NoOpDebugDataDumper.java
@@ -23,7 +23,7 @@
 public class NoOpDebugDataDumper extends DebugDataDumper {
 
   public NoOpDebugDataDumper() {
-    super(Path.of("."), true);
+    super(Path.of("."));
   }
 
   @Override

diff --git a/...etransition/src/test/java/tech/pegasys/teku/statetransition/util/DebugDataDumperTest.java b/...etransition/src/test/java/tech/pegasys/teku/statetransition/util/DebugDataDumperTest.java
@@ -20,7 +20,6 @@
 import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
-import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
 import java.sql.Date;
 import java.text.DateFormat;
@@ -44,7 +43,7 @@ class DebugDataDumperTest {
 
   @Test
   void saveGossipMessageDecodingError_shouldSaveToFile(@TempDir Path tempDir) {
-    final DebugDataDumper manager = new DebugDataDumper(tempDir, true);
+    final DebugDataDumper manager = new DebugDataDumper(tempDir);
     final Bytes messageBytes = dataStructureUtil.stateBuilderPhase0().build().sszSerialize();
     final Optional<UInt64> arrivalTimestamp = Optional.of(timeProvider.getTimeInMillis());
     manager.saveGossipMessageDecodingError(
@@ -61,29 +60,9 @@ void saveGossipMessageDecodingError_shouldSaveToFile(@TempDir Path tempDir) {
     checkBytesSavedToFile(expectedFile, messageBytes);
   }
 
-  @Test
-  void saveGossipMessageDecodingError_shouldNotSaveToFileWhenDisabled(@TempDir Path tempDir) {
-    final DebugDataDumper manager = new DebugDataDumper(tempDir, false);
-    final Bytes messageBytes = dataStructureUtil.stateBuilderPhase0().build().sszSerialize();
-    final Optional<UInt64> arrivalTimestamp = Optional.of(timeProvider.getTimeInMillis());
-    manager.saveGossipMessageDecodingError(
-        "/eth/test/topic", arrivalTimestamp, messageBytes, new Throwable());
-    assertThat(manager.isEnabled()).isFalse();
-
-    final String fileName =
-        String.format("%s.ssz", formatTimestamp(timeProvider.getTimeInMillis().longValue()));
-    final Path expectedFile =
-        tempDir
-            .resolve("gossip_messages")
-            .resolve("decoding_error")
-            .resolve("_eth_test_topic")
-            .resolve(fileName);
-    checkFileNotExist(expectedFile);
-  }
-
   @Test
   void saveGossipRejectedMessageToFile_shouldSaveToFile(@TempDir Path tempDir) {
-    final DebugDataDumper manager = new DebugDataDumper(tempDir, true);
+    final DebugDataDumper manager = new DebugDataDumper(tempDir);
     final Bytes messageBytes = dataStructureUtil.stateBuilderPhase0().build().sszSerialize();
     final Optional<UInt64> arrivalTimestamp = Optional.of(timeProvider.getTimeInMillis());
     manager.saveGossipRejectedMessageToFile(
@@ -100,28 +79,9 @@ void saveGossipRejectedMessageToFile_shouldSaveToFile(@TempDir Path tempDir) {
     checkBytesSavedToFile(expectedFile, messageBytes);
   }
 
-  @Test
-  void saveGossipRejectedMessageToFile_shouldNotSaveToFileWhenDisabled(@TempDir Path tempDir) {
-    final DebugDataDumper manager = new DebugDataDumper(tempDir, false);
-    final Bytes messageBytes = dataStructureUtil.stateBuilderPhase0().build().sszSerialize();
-    final Optional<UInt64> arrivalTimestamp = Optional.of(timeProvider.getTimeInMillis());
-    manager.saveGossipRejectedMessageToFile(
-        "/eth/test/topic", arrivalTimestamp, messageBytes, Optional.of("reason"));
-    assertThat(manager.isEnabled()).isFalse();
-
-    final String fileName = String.format("%s.ssz", arrivalTimestamp);
-    final Path expectedFile =
-        tempDir
-            .resolve("gossip_messages")
-            .resolve("rejected")
-            .resolve("_eth_test_topic")
-            .resolve(fileName);
-    checkFileNotExist(expectedFile);
-  }
-
   @Test
   void saveInvalidBlockToFile_shouldSaveToFile(@TempDir Path tempDir) {
-    final DebugDataDumper manager = new DebugDataDumper(tempDir, true);
+    final DebugDataDumper manager = new DebugDataDumper(tempDir);
     final SignedBeaconBlock block = dataStructureUtil.randomSignedBeaconBlock();
     manager.saveInvalidBlockToFile(
         block.getSlot(),
@@ -136,23 +96,9 @@ void saveInvalidBlockToFile_shouldSaveToFile(@TempDir Path tempDir) {
     checkBytesSavedToFile(expectedFile, block.sszSerialize());
   }
 
-  @Test
-  void saveInvalidBlockToFile_shouldNotSaveToFileWhenDisabled(@TempDir Path tempDir) {
-    final DebugDataDumper manager = new DebugDataDumper(tempDir, false);
-    final SignedBeaconBlock block = dataStructureUtil.randomSignedBeaconBlock();
-    manager.saveInvalidBlockToFile(
-        block.getSlot(), block.getRoot(), block.sszSerialize(), "reason", Optional.empty());
-    assertThat(manager.isEnabled()).isFalse();
-
-    final String fileName =
-        String.format("%s_%s.ssz", block.getSlot(), block.getRoot().toUnprefixedHexString());
-    final Path expectedFile = tempDir.resolve("invalid_blocks").resolve(fileName);
-    checkFileNotExist(expectedFile);
-  }
-
   @Test
   void saveBytesToFile_shouldNotThrowExceptionWhenNoDirectory(@TempDir Path tempDir) {
-    final DebugDataDumper manager = new DebugDataDumper(tempDir, true);
+    final DebugDataDumper manager = new DebugDataDumper(tempDir);
     assertDoesNotThrow(
         () -> {
           final boolean success =
@@ -165,7 +111,7 @@ void saveBytesToFile_shouldNotThrowExceptionWhenNoDirectory(@TempDir Path tempDi
   @Test
   @DisabledOnOs(OS.WINDOWS) // Can't set permissions on Windows
   void saveBytesToFile_shouldNotEscalateWhenIOException(@TempDir Path tempDir) {
-    final DebugDataDumper manager = new DebugDataDumper(tempDir, true);
+    final DebugDataDumper manager = new DebugDataDumper(tempDir);
     final File invalidPath = tempDir.resolve("invalid").toFile();
     assertThat(invalidPath.mkdirs()).isTrue();
     assertThat(invalidPath.setWritable(false)).isTrue();
@@ -182,13 +128,13 @@ void saveBytesToFile_shouldNotEscalateWhenIOException(@TempDir Path tempDir) {
   @DisabledOnOs(OS.WINDOWS) // Can't set permissions on Windows
   void constructionOfDirectories_shouldDisableWhenFailedToCreate(@TempDir Path tempDir) {
     assertThat(tempDir.toFile().setWritable(false)).isTrue();
-    final DebugDataDumper manager = new DebugDataDumper(tempDir, true);
+    final DebugDataDumper manager = new DebugDataDumper(tempDir);
     assertThat(manager.isEnabled()).isFalse();
   }
 
   @Test
   void formatOptionalTimestamp_shouldFormatTimestamp(@TempDir Path tempDir) {
-    final DebugDataDumper manager = new DebugDataDumper(tempDir, true);
+    final DebugDataDumper manager = new DebugDataDumper(tempDir);
     final String formattedTimestamp =
         manager.formatOptionalTimestamp(Optional.of(timeProvider.getTimeInMillis()), timeProvider);
     assertThat(formattedTimestamp)
@@ -197,7 +143,7 @@ void formatOptionalTimestamp_shouldFormatTimestamp(@TempDir Path tempDir) {
 
   @Test
   void formatOptionalTimestamp_shouldGenerateTimestamp(@TempDir Path tempDir) {
-    final DebugDataDumper manager = new DebugDataDumper(tempDir, true);
+    final DebugDataDumper manager = new DebugDataDumper(tempDir);
     final String formattedTimestamp =
         manager.formatOptionalTimestamp(Optional.empty(), timeProvider);
     assertThat(formattedTimestamp)
@@ -213,17 +159,6 @@ private void checkBytesSavedToFile(final Path path, final Bytes expectedBytes) {
     }
   }
 
-  private void checkFileNotExist(final Path path) {
-    try {
-      Bytes.wrap(Files.readAllBytes(path));
-    } catch (IOException e) {
-      if (e instanceof NoSuchFileException) {
-        return;
-      }
-    }
-    fail("File was found and bytes read");
-  }
-
   private String formatTimestamp(final long timeInMillis) {
     final DateFormat df = new SimpleDateFormat("yyyy-MM-dd'T'HH_mm_ss.SS");
     final Date date = new Date(timeInMillis);

diff --git a/...aconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java b/...aconchain/src/main/java/tech/pegasys/teku/services/beaconchain/BeaconChainController.java
@@ -153,6 +153,7 @@
 import tech.pegasys.teku.statetransition.util.FutureItems;
 import tech.pegasys.teku.statetransition.util.PendingPool;
 import tech.pegasys.teku.statetransition.util.PoolFactory;
+import tech.pegasys.teku.statetransition.util.noop.NoOpDebugDataDumper;
 import tech.pegasys.teku.statetransition.validation.AggregateAttestationValidator;
 import tech.pegasys.teku.statetransition.validation.AttestationValidator;
 import tech.pegasys.teku.statetransition.validation.AttesterSlashingValidator;
@@ -321,9 +322,9 @@ public BeaconChainController(
         eventChannels.getPublisher(ReceivedBlockEventsChannel.class);
     this.forkChoiceExecutor = new AsyncRunnerEventThread("forkchoice", asyncRunnerFactory);
     this.debugDataDumper =
-        new DebugDataDumper(
-            serviceConfig.getDataDirLayout().getDebugDataDirectory(),
-            beaconConfig.p2pConfig().isP2pDumpsToFileEnabled());
+        beaconConfig.p2pConfig().isP2pDumpsToFileEnabled()
+            ? new DebugDataDumper(serviceConfig.getDataDirLayout().getDebugDataDirectory())
+            : new NoOpDebugDataDumper();
     this.futureItemsMetric =
         SettableLabelledGauge.create(
             metricsSystem,