Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Normalize metrics from measurements #45

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Normalize metrics from measurements #45

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
23cd790
Add metric attributes to measurements in the driver
nineinchnick Sep 12, 2022
a75646c
Save metrics normalized from measurements
nineinchnick Sep 13, 2022
87c507a
Document metrics in the data model
nineinchnick Sep 14, 2022
0f2f138
Use advisory locks to avoid duplicate metrics
nineinchnick Sep 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions benchto-driver/src/main/java/io/trino/benchto/driver/graphite/GraphiteMetricsLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*/
package io.trino.benchto.driver.graphite;

import com.google.common.collect.ImmutableMap;
import io.trino.benchto.driver.Measurable;
import io.trino.benchto.driver.execution.BenchmarkExecutionResult;
import io.trino.benchto.driver.execution.ExecutionSynchronizer;
Expand All @@ -32,6 +33,7 @@

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add description in commit message

import java.time.ZonedDateTime;
import java.time.temporal.ChronoUnit;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
Expand All @@ -49,6 +51,7 @@
public class GraphiteMetricsLoader
implements PostExecutionMeasurementProvider
{
private static final String METRIC_SCOPE = "cluster";
nineinchnick marked this conversation as resolved.
Show resolved Hide resolved
private static final Logger LOG = LoggerFactory.getLogger(GraphiteMetricsLoader.class);

@Autowired
Expand Down Expand Up @@ -126,7 +129,7 @@ private List<Measurement> doLoadMeasurements(long fromEpochSecond, long toEpochS
if (metricValues.length > 0) {
// last non zero measurement contains total over time
double totalBytes = getLastValueGreaterThanZero(metricValues);
measurements.add(Measurement.measurement("cluster-network_total", "BYTES", totalBytes));
measurements.add(Measurement.measurement("network", "BYTES", totalBytes, ImmutableMap.of("scope", METRIC_SCOPE, "aggregate", "total")));
}
}
return measurements;
Expand All @@ -150,8 +153,8 @@ private void addMeanMaxMeasurements(Map<String, double[]> loadedMetrics, List<Me
{
Optional<StatisticalSummary> statistics = getStats(loadedMetrics, metricName);
if (statistics.isPresent()) {
measurements.add(Measurement.measurement("cluster-" + metricName + "_max", unit, statistics.get().getMax()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

won't this make scripts harder, e.g: previously I could just select name in SQL query. Now I will have to join on attributes and do subquery. Seems complicated

measurements.add(Measurement.measurement("cluster-" + metricName + "_mean", unit, statistics.get().getMean()));
measurements.add(Measurement.measurement(metricName, unit, statistics.get().getMax(), ImmutableMap.of("scope", "cluster", "aggregate", "max")));
nineinchnick marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

METRIC_SCOPE instead of cluster?

measurements.add(Measurement.measurement(metricName, unit, statistics.get().getMean(), ImmutableMap.of("scope", "cluster", "aggregate", "mean")));
}
}

Expand Down
3 changes: 2 additions & 1 deletion benchto-driver/src/main/java/io/trino/benchto/driver/presto/PrestoClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import javax.measure.unit.Unit;

import java.net.URI;
import java.util.Collections;
import java.util.List;
import java.util.Map;

Expand Down Expand Up @@ -123,7 +124,7 @@ private URI buildQueryInfoURI(String queryId)
private Measurement parseQueryStatistic(String name, Object statistic, Unit requiredUnit)
{
double value = UnitConverter.parseValueAsUnit(statistic.toString(), requiredUnit);
return measurement("prestoQuery-" + name, UnitConverter.format(requiredUnit), value);
return measurement(name, UnitConverter.format(requiredUnit), value, Collections.singletonMap("scope", "prestoQuery"));
nineinchnick marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prestoQuery -> query

}

@SuppressWarnings("unused")
Expand Down
15 changes: 14 additions & 1 deletion benchto-driver/src/main/java/io/trino/benchto/driver/service/Measurement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@

import com.fasterxml.jackson.annotation.JsonAutoDetect;

import java.util.Collections;
import java.util.Map;

import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.ANY;
import static com.google.common.base.MoreObjects.toStringHelper;
import static java.util.Objects.requireNonNull;
Expand All @@ -26,12 +29,20 @@ public class Measurement
private String unit;
private double value;

private Map<String, String> attributes;

public static Measurement measurement(String name, String unit, double value)
{
return measurement(name, unit, value, Collections.emptyMap());
}

public static Measurement measurement(String name, String unit, double value, Map<String, String> attributes)
{
Measurement measurement = new Measurement();
measurement.name = requireNonNull(name);
measurement.unit = requireNonNull(unit);
measurement.value = value;
measurement.attributes = attributes;
return measurement;
}

Expand All @@ -47,7 +58,7 @@ public boolean equals(Object o)

Measurement that = (Measurement) o;

return Double.compare(that.value, value) == 0 && name.equals(that.name) && unit.equals(that.unit);
return Double.compare(that.value, value) == 0 && name.equals(that.name) && unit.equals(that.unit) && attributes.equals(that.attributes);
}

@Override
Expand All @@ -59,6 +70,7 @@ public int hashCode()
result = 31 * result + unit.hashCode();
temp = Double.doubleToLongBits(value);
result = 31 * result + (int) (temp ^ (temp >>> 32));
result = 31 * result + attributes.hashCode();
return result;
}

Expand All @@ -69,6 +81,7 @@ public String toString()
.add("name", name)
.add("unit", unit)
.add("value", value)
.add("attributes", attributes)
.toString();
}
}
29 changes: 16 additions & 13 deletions benchto-driver/src/test/java/io/trino/benchto/driver/PrestoClientIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@
import org.springframework.beans.factory.annotation.Autowired;

import java.io.IOException;
import java.util.Collections;
import java.util.List;
import java.util.Map;

import static org.assertj.core.api.Assertions.assertThat;
import static org.springframework.http.MediaType.APPLICATION_JSON;
Expand All @@ -44,20 +46,21 @@ public void testPrestoClientLoadMetrics()

List<Measurement> measurements = prestoClient.loadMetrics("test_query_id");

Map<String, String> attributes = Collections.singletonMap("scope", "prestoQuery");
assertThat(measurements).containsExactly(
Measurement.measurement("prestoQuery-analysisTime", "MILLISECONDS", 21.07),
Measurement.measurement("prestoQuery-planningTime", "MILLISECONDS", 24.72),
Measurement.measurement("prestoQuery-totalScheduledTime", "MILLISECONDS", 66000.0),
Measurement.measurement("prestoQuery-totalCpuTime", "MILLISECONDS", 63600.0),
Measurement.measurement("prestoQuery-totalBlockedTime", "MILLISECONDS", 287400.0),
Measurement.measurement("prestoQuery-finishingTime", "MILLISECONDS", 69000.0),
Measurement.measurement("prestoQuery-rawInputDataSize", "BYTES", 1.34E9),
Measurement.measurement("prestoQuery-processedInputDataSize", "BYTES", 7.3961E8),
Measurement.measurement("prestoQuery-internalNetworkInputDataSize", "BYTES", 7.2961E8),
Measurement.measurement("prestoQuery-physicalInputDataSize", "BYTES", 1.35E9),
Measurement.measurement("prestoQuery-outputDataSize", "BYTES", 6900.0),
Measurement.measurement("prestoQuery-peakTotalMemoryReservation", "BYTES", 6800.0),
Measurement.measurement("prestoQuery-physicalWrittenDataSize", "BYTES", 462265065.0));
Measurement.measurement("analysisTime", "MILLISECONDS", 21.07, attributes),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will break scripts (jupyther). I think it's fine, but needs to be announced

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not only scripts but our workflows as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean alerts?

Measurement.measurement("planningTime", "MILLISECONDS", 24.72, attributes),
Measurement.measurement("totalScheduledTime", "MILLISECONDS", 66000.0, attributes),
Measurement.measurement("totalCpuTime", "MILLISECONDS", 63600.0, attributes),
Measurement.measurement("totalBlockedTime", "MILLISECONDS", 287400.0, attributes),
Measurement.measurement("finishingTime", "MILLISECONDS", 69000.0, attributes),
Measurement.measurement("rawInputDataSize", "BYTES", 1.34E9, attributes),
Measurement.measurement("processedInputDataSize", "BYTES", 7.3961E8, attributes),
Measurement.measurement("internalNetworkInputDataSize", "BYTES", 7.2961E8, attributes),
Measurement.measurement("physicalInputDataSize", "BYTES", 1.35E9, attributes),
Measurement.measurement("outputDataSize", "BYTES", 6900.0, attributes),
Measurement.measurement("peakTotalMemoryReservation", "BYTES", 6800.0, attributes),
Measurement.measurement("physicalWrittenDataSize", "BYTES", 462265065.0, attributes));

restServiceServer.verify();
}
Expand Down