Response to additional code review comments from
MODIFY
testing-overhead/src/test/java/io/opentelemetry/agents/AgentResolver.java
testing-overhead/src/test/java/io/opentelemetry/agents/AgentResolver.java
@@ -8,8 +8,8 @@ import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
-import java.nio.file.StandardOpenOption;
import java.nio.file.StandardCopyOption;
+import java.nio.file.StandardOpenOption;
import java.util.Optional;
import okhttp3.OkHttpClient;
import okhttp3.Request;

MODIFY
testing-overhead/src/test/java/io/opentelemetry/results/AppPerfResults.java
testing-overhead/src/test/java/io/opentelemetry/results/AppPerfResults.java
@@ -9,18 +9,18 @@ import io.opentelemetry.config.TestConfig;
public class AppPerfResults {
- private final Agent agent;
- private final TestConfig config;
- private final double iterationAvg;
- private final double iterationP95;
- private final double requestAvg;
- private final double requestP95;
- private final long totalGCTime;
- private final long totalAllocated;
- private final MinMax heapUsed;
- private final float maxThreadContextSwitchRate;
- private final long startupDurationMs;
- private final long peakThreadCount;
+ final Agent agent;
+ final TestConfig config;
+ final double iterationAvg;
+ final double iterationP95;
+ final double requestAvg;
+ final double requestP95;
+ final long totalGCTime;
+ final long totalAllocated;
+ final MinMax heapUsed;
+ final float maxThreadContextSwitchRate;
+ final long startupDurationMs;
+ final long peakThreadCount;
private AppPerfResults(Builder builder) {
this.agent = builder.agent;
@@ -37,62 +37,13 @@ public class AppPerfResults {
this.peakThreadCount = builder.peakThreadCount;
}
- Agent getAgent() {
- return agent;
- }
-
- String getAgentName() {
- return agent.getName();
- }
-
- TestConfig getConfig() {
- return config;
- }
-
- double getIterationAvg() {
- return iterationAvg;
- }
-
- double getIterationP95() {
- return iterationP95;
- }
-
- double getRequestAvg() {
- return requestAvg;
- }
-
- double getRequestP95() {
- return requestP95;
- }
-
- long getTotalGCTime() {
- return totalGCTime;
- }
-
- long getTotalAllocated() {
- return totalAllocated;
- }
-
double getTotalAllocatedMB() {
return totalAllocated / (1024.0 * 1024.0);
}
- MinMax getHeapUsed() {
- return heapUsed;
- }
-
- float getMaxThreadContextSwitchRate() {
- return maxThreadContextSwitchRate;
- }
-
- long getStartupDurationMs() {
- return startupDurationMs;
- }
-
- public long getPeakThreadCount() {
- return peakThreadCount;
+ String getAgentName() {
+ return agent.getName();
}
-
static Builder builder() {
return new Builder();
}

MODIFY
testing-overhead/src/test/java/io/opentelemetry/results/ConsoleResultsPersister.java
testing-overhead/src/test/java/io/opentelemetry/results/ConsoleResultsPersister.java
*/
package io.opentelemetry.results;
-import io.opentelemetry.agents.Agent;
import io.opentelemetry.config.TestConfig;
import java.util.Comparator;
import java.util.List;
@@ -14,25 +13,25 @@ public class ConsoleResultsPersister implements ResultsPersister {
@Override
public void write(List<AppPerfResults> results) {
- TestConfig config = results.stream().findFirst().get().getConfig();
+ TestConfig config = results.stream().findFirst().get().config;
System.out.println("----------------------------------------------------------");
System.out.printf(" %s : %s\n", config.getName(), config.getDescription());
System.out.printf(" %d users, %d iterations\n", config.getConcurrentConnections(), config.getTotalIterations());
System.out.println("----------------------------------------------------------");
- display(results, "Agent", appPerfResults -> appPerfResults.getAgent().getName());
- display(results, "Startup time (ms)", res -> String.valueOf(res.getStartupDurationMs()));
+ display(results, "Agent", appPerfResults -> appPerfResults.agent.getName());
+ display(results, "Startup time (ms)", res -> String.valueOf(res.startupDurationMs));
display(results, "Total allocated MB", res -> format(res.getTotalAllocatedMB()));
- display(results, "Heap (min)", res -> String.valueOf(res.getHeapUsed().min));
- display(results, "Heap (max)", res -> String.valueOf(res.getHeapUsed().max));
+ display(results, "Heap (min)", res -> String.valueOf(res.heapUsed.min));
+ display(results, "Heap (max)", res -> String.valueOf(res.heapUsed.max));
display(results, "Thread switch rate",
- res -> String.valueOf(res.getMaxThreadContextSwitchRate()));
- display(results, "GC time", res -> String.valueOf(res.getTotalGCTime()));
- display(results, "Req. mean", res -> format(res.getRequestAvg()));
- display(results, "Req. p95", res -> format(res.getRequestP95()));
- display(results, "Iter. mean", res -> format(res.getIterationAvg()));
- display(results, "Iter. p95", res -> format(res.getIterationP95()));
- display(results, "Peak threads", res -> String.valueOf(res.getPeakThreadCount()));
+ res -> String.valueOf(res.maxThreadContextSwitchRate));
+ display(results, "GC time", res -> String.valueOf(res.totalGCTime));
+ display(results, "Req. mean", res -> format(res.requestAvg));
+ display(results, "Req. p95", res -> format(res.requestP95));
+ display(results, "Iter. mean", res -> format(res.iterationAvg));
+ display(results, "Iter. p95", res -> format(res.iterationP95));
+ display(results, "Peak threads", res -> String.valueOf(res.peakThreadCount));
}
private void display(List<AppPerfResults> results, String pref,
@@ -41,7 +40,6 @@ public class ConsoleResultsPersister implements ResultsPersister {
results.stream()
.sorted(Comparator.comparing(AppPerfResults::getAgentName))
.forEach(result -> {
- Agent agent = result.getAgent();
System.out.printf("%17s", vs.apply(result));
});
System.out.println();

MODIFY
testing-overhead/src/test/java/io/opentelemetry/results/ResultsCollector.java
testing-overhead/src/test/java/io/opentelemetry/results/ResultsCollector.java
@@ -84,11 +84,11 @@ public class ResultsCollector {
}
private long readTotalGCTime(Path jfrFile) throws IOException {
- return JFRUtils.totalLongEvents(jfrFile, "jdk.G1GarbageCollection", "duration");
+ return JFRUtils.sumLongEventValues(jfrFile, "jdk.G1GarbageCollection", "duration");
}
private long readTotalAllocated(Path jfrFile) throws IOException {
- return JFRUtils.totalLongEvents(jfrFile, "jdk.ThreadAllocationStatistics", "allocated");
+ return JFRUtils.sumLongEventValues(jfrFile, "jdk.ThreadAllocationStatistics", "allocated");
}
private MinMax readHeapUsed(Path jfrFile) throws IOException {

MODIFY
testing-overhead/src/test/java/io/opentelemetry/util/JFRUtils.java
testing-overhead/src/test/java/io/opentelemetry/util/JFRUtils.java
*/
package io.opentelemetry.util;
+import io.opentelemetry.results.AppPerfResults.MinMax;
import java.io.IOException;
import java.nio.file.Path;
import java.util.function.BiFunction;
-import io.opentelemetry.results.AppPerfResults.MinMax;
import jdk.jfr.consumer.RecordedEvent;
import jdk.jfr.consumer.RecordingFile;
public class JFRUtils {
- public static long totalLongEvents(Path jfrFile, String eventName,
+ public static long sumLongEventValues(Path jfrFile, String eventName,
String longValueKey) throws IOException {
return reduce(jfrFile, eventName, longValueKey, 0L, Long::sum);
}

MODIFY
testing-overhead/src/test/java/io/opentelemetry/util/NamingConvention.java
testing-overhead/src/test/java/io/opentelemetry/util/NamingConvention.java
@@ -8,26 +8,43 @@ import io.opentelemetry.agents.Agent;
import java.nio.file.Path;
import java.nio.file.Paths;
+/**
+ * This utility class provides the standard file naming conventions, primarily
+ * for files that are shared between containers and the test runner. It consolidates
+ * the naming logic into one place to ensure consistency, reduce duplication, and
+ * decrease errors.
+ */
public class NamingConvention {
private final String dir;
- public NamingConvention(){
- this(".");
- }
-
public NamingConvention(String dir) {this.dir = dir;}
+ /**
+ * Returns a path to the location of the k6 results json file.
+ * @param agent The agent to get results file path for
+ */
public Path k6Results(Agent agent){
return Paths.get(dir, "k6_out_" + agent.getName() + ".json");
}
+ /**
+ * Returns a path to the location of the jfr output file for a given agent run.
+ * @param agent The agent to get the jfr file path for.
+ */
public Path jfrFile(Agent agent) {
return Paths.get(dir, "petclinic-" + agent.getName() + ".jfr");
}
+ /**
+ * Returns the path to the file that contains the startup duration for a given agent run.
+ * @param agent The agent to get the startup duration for.
+ */
public Path startupDurationFile(Agent agent) { return Paths.get(dir, "startup-time-" + agent.getName() + ".txt"); }
+ /**
+ * Returns the root path that this naming convention was configured with.
+ */
public String root() {
return dir;
}

MODIFY
testing-overhead/src/test/java/io/opentelemetry/util/NamingConventions.java
testing-overhead/src/test/java/io/opentelemetry/util/NamingConventions.java
package io.opentelemetry.util;
+/**
+ * An container to hold both the local and container naming conventions.
+ */
public class NamingConventions {
public final NamingConvention container = new NamingConvention("/results");
public final NamingConvention local = new NamingConvention(".");
+ /**
+ * @return Root path for the local naming convention (where results are output)
+ */
public String localResults() {
return local.root();
}
+ /**
+ * @return Root path for the container naming convention (where results are output)
+ */
public String containerResults() {
return container.root();
}

Response to additional code review comments from #3648. (#3761)