Skip to content

Commit

Permalink
Refactor ContainerResource to avoid using nulls
Browse files Browse the repository at this point in the history
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#6694
Issue-Id: open-telemetry/opentelemetry-java#2337
  • Loading branch information
edysli committed Oct 15, 2022
1 parent b25283d commit 58990bb
Showing 1 changed file with 22 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,11 @@
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 javax.annotation.Nonnull;
import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement;

/** Factory for {@link Resource} retrieving Container ID information. */
Expand All @@ -35,13 +34,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. */
Expand All @@ -57,33 +54,30 @@ public static Resource get() {
* @return containerId
*/
@IgnoreJRERequirement
@Nullable
private static String extractContainerId(Path cgroupFilePath) {
@Nonnull
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) {
@Nonnull
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;
Expand All @@ -105,16 +99,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();
}
}

Expand Down

0 comments on commit 58990bb

Please sign in to comment.