From 860c7160a29031f54c119a75f272f594f5a8b519 Mon Sep 17 00:00:00 2001
From: Etienne Dysli Metref <etienne.dysli@gmail.com>
Date: Sun, 16 Oct 2022 00:02:22 +0200
Subject: [PATCH] Refactor ContainerResource to avoid using nulls

Refactor class `ContainerResource` so that its methods return
`Optional`s instead of `null`. This makes for safer code that doesn't
rely on `null` to signal the absence of result. Moreover,
`Optional.map()` and `Optional.orElseGet()` use the same functional
style as `Stream`, which is well readable.

Issue-Id: open-telemetry/opentelemetry-java-instrumentation#6694
Issue-Id: open-telemetry/opentelemetry-java#2337
---
 .../resources/ContainerResource.java          | 47 ++++++++-----------
 1 file changed, 19 insertions(+), 28 deletions(-)

diff --git a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ContainerResource.java b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ContainerResource.java
index df988e7e0ac2..03040aa9c9da 100644
--- a/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ContainerResource.java
+++ b/instrumentation/resources/library/src/main/java/io/opentelemetry/instrumentation/resources/ContainerResource.java
@@ -12,12 +12,10 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Stream;
-import javax.annotation.Nullable;
 import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement;
 
 /** Factory for {@link Resource} retrieving Container ID information. */
@@ -35,13 +33,11 @@ private static Resource buildSingleton(String uniqueHostNameFileName) {
 
   // package private for testing
   static Resource buildResource(Path path) {
-    String containerId = extractContainerId(path);
-
-    if (containerId == null || containerId.isEmpty()) {
-      return Resource.empty();
-    } else {
-      return Resource.create(Attributes.of(ResourceAttributes.CONTAINER_ID, containerId));
-    }
+    return extractContainerId(path)
+        .map(
+            containerId ->
+                Resource.create(Attributes.of(ResourceAttributes.CONTAINER_ID, containerId)))
+        .orElseGet(Resource::empty);
   }
 
   /** Returns resource with container information. */
@@ -57,33 +53,28 @@ public static Resource get() {
    * @return containerId
    */
   @IgnoreJRERequirement
-  @Nullable
-  private static String extractContainerId(Path cgroupFilePath) {
+  private static Optional<String> extractContainerId(Path cgroupFilePath) {
     if (!Files.exists(cgroupFilePath) || !Files.isReadable(cgroupFilePath)) {
-      return null;
+      return Optional.empty();
     }
     try (Stream<String> lines = Files.lines(cgroupFilePath)) {
-      Optional<String> value =
-          lines
-              .filter(line -> !line.isEmpty())
-              .map(ContainerResource::getIdFromLine)
-              .filter(Objects::nonNull)
-              .findFirst();
-      if (value.isPresent()) {
-        return value.get();
-      }
+      return lines
+          .filter(line -> !line.isEmpty())
+          .map(ContainerResource::getIdFromLine)
+          .filter(Optional::isPresent)
+          .findFirst()
+          .orElse(Optional.empty());
     } catch (Exception e) {
       logger.log(Level.WARNING, "Unable to read file", e);
     }
-    return null;
+    return Optional.empty();
   }
 
-  @Nullable
-  private static String getIdFromLine(String line) {
+  private static Optional<String> getIdFromLine(String line) {
     // This cgroup output line should have the container id in it
     int lastSlashIdx = line.lastIndexOf('/');
     if (lastSlashIdx < 0) {
-      return null;
+      return Optional.empty();
     }
 
     String containerId;
@@ -105,16 +96,16 @@ private static String getIdFromLine(String line) {
         endIdx = lastSection.length();
       }
       if (startIdx > endIdx) {
-        return null;
+        return Optional.empty();
       }
 
       containerId = lastSection.substring(startIdx, endIdx);
     }
 
     if (OtelEncodingUtils.isValidBase16String(containerId) && !containerId.isEmpty()) {
-      return containerId;
+      return Optional.of(containerId);
     } else {
-      return null;
+      return Optional.empty();
     }
   }