Skip to content

Commit

Permalink
Code review fulfillment (part 2)
Browse files Browse the repository at this point in the history
Signed-off-by: Fabian Stäber <fabian@fstab.de>
  • Loading branch information
fstab committed Dec 17, 2023
1 parent fd073f9 commit 79717ba
Show file tree
Hide file tree
Showing 6 changed files with 199 additions and 199 deletions.
2 changes: 1 addition & 1 deletion dependencyManagement/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ val DEPENDENCIES = listOf(
"com.google.guava:guava-beta-checker:1.0",
"com.sun.net.httpserver:http:20070405",
"com.tngtech.archunit:archunit-junit5:1.2.1",
"io.prometheus:prometheus-metrics-exporter-httpserver:1.1.0",
"com.uber.nullaway:nullaway:0.10.18",
"edu.berkeley.cs.jqf:jqf-fuzz:1.7", // jqf-fuzz version 1.8+ requires Java 11+
"eu.rekawek.toxiproxy:toxiproxy-java:2.1.7",
Expand All @@ -70,6 +69,7 @@ val DEPENDENCIES = listOf(
"io.opentelemetry.contrib:opentelemetry-aws-xray-propagator:1.29.0-alpha",
"io.opentracing:opentracing-api:0.33.0",
"io.opentracing:opentracing-noop:0.33.0",
"io.prometheus:prometheus-metrics-exporter-httpserver:1.1.0",
"junit:junit:4.13.2",
"nl.jqno.equalsverifier:equalsverifier:3.15.4",
"org.awaitility:awaitility:4.2.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
import io.prometheus.metrics.model.snapshots.SummarySnapshot.SummaryDataPointSnapshot;
import io.prometheus.metrics.model.snapshots.Unit;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -115,6 +114,10 @@ MetricSnapshots convert(@Nullable Collection<MetricData> metricDataCollection) {

@Nullable
private MetricSnapshot convert(MetricData metricData) {

// Note that AggregationTemporality.DELTA should never happen
// because PrometheusMetricReader#getAggregationTemporality returns CUMULATIVE.

MetricMetadata metadata = convertMetadata(metricData);
InstrumentationScopeInfo scope = metricData.getInstrumentationScopeInfo();
switch (metricData.getType()) {
Expand Down Expand Up @@ -165,105 +168,106 @@ private GaugeSnapshot convertLongGauge(
MetricMetadata metadata,
InstrumentationScopeInfo scope,
Collection<LongPointData> dataPoints) {
GaugeDataPointSnapshot[] data = new GaugeDataPointSnapshot[dataPoints.size()];
int i = 0;
List<GaugeDataPointSnapshot> data = new ArrayList<>(dataPoints.size());
for (LongPointData longData : dataPoints) {
data[i++] =
data.add(
new GaugeDataPointSnapshot(
(double) longData.getValue(),
convertAttributes(scope, longData.getAttributes()),
convertLongExemplar(longData.getExemplars()));
convertLongExemplar(longData.getExemplars())));
}
return new GaugeSnapshot(metadata, Arrays.asList(data));
return new GaugeSnapshot(metadata, data);
}

private CounterSnapshot convertLongCounter(
MetricMetadata metadata,
InstrumentationScopeInfo scope,
Collection<LongPointData> dataPoints) {
CounterDataPointSnapshot[] data = new CounterDataPointSnapshot[dataPoints.size()];
int i = 0;
List<CounterDataPointSnapshot> data =
new ArrayList<CounterDataPointSnapshot>(dataPoints.size());
for (LongPointData longData : dataPoints) {
data[i++] =
data.add(
new CounterDataPointSnapshot(
(double) longData.getValue(),
convertAttributes(scope, longData.getAttributes()),
convertLongExemplar(longData.getExemplars()),
longData.getStartEpochNanos() / NANOS_PER_MILLISECOND);
longData.getStartEpochNanos() / NANOS_PER_MILLISECOND));
}
return new CounterSnapshot(metadata, Arrays.asList(data));
return new CounterSnapshot(metadata, data);
}

private GaugeSnapshot convertDoubleGauge(
MetricMetadata metadata,
InstrumentationScopeInfo scope,
Collection<DoublePointData> dataPoints) {
GaugeDataPointSnapshot[] data = new GaugeDataPointSnapshot[dataPoints.size()];
int i = 0;
List<GaugeDataPointSnapshot> data = new ArrayList<>(dataPoints.size());
for (DoublePointData doubleData : dataPoints) {
data[i++] =
data.add(
new GaugeDataPointSnapshot(
doubleData.getValue(),
convertAttributes(scope, doubleData.getAttributes()),
convertDoubleExemplar(doubleData.getExemplars()));
convertDoubleExemplar(doubleData.getExemplars())));
}
return new GaugeSnapshot(metadata, Arrays.asList(data));
return new GaugeSnapshot(metadata, data);
}

private CounterSnapshot convertDoubleCounter(
MetricMetadata metadata,
InstrumentationScopeInfo scope,
Collection<DoublePointData> dataPoints) {
CounterDataPointSnapshot[] data = new CounterDataPointSnapshot[dataPoints.size()];
int i = 0;
List<CounterDataPointSnapshot> data = new ArrayList<>(dataPoints.size());
for (DoublePointData doubleData : dataPoints) {
data[i++] =
data.add(
new CounterDataPointSnapshot(
doubleData.getValue(),
convertAttributes(scope, doubleData.getAttributes()),
convertDoubleExemplar(doubleData.getExemplars()),
doubleData.getStartEpochNanos() / NANOS_PER_MILLISECOND);
doubleData.getStartEpochNanos() / NANOS_PER_MILLISECOND));
}
return new CounterSnapshot(metadata, Arrays.asList(data));
return new CounterSnapshot(metadata, data);
}

private HistogramSnapshot convertHistogram(
MetricMetadata metadata,
InstrumentationScopeInfo scope,
Collection<HistogramPointData> dataPoints) {
HistogramDataPointSnapshot[] data = new HistogramDataPointSnapshot[dataPoints.size()];
int i = 0;
List<HistogramDataPointSnapshot> data = new ArrayList<>(dataPoints.size());
for (HistogramPointData histogramData : dataPoints) {
List<Double> boundaries = new ArrayList<>(histogramData.getBoundaries().size() + 1);
boundaries.addAll(histogramData.getBoundaries());
boundaries.add(Double.POSITIVE_INFINITY);
data[i++] =
data.add(
new HistogramDataPointSnapshot(
ClassicHistogramBuckets.of(boundaries, histogramData.getCounts()),
histogramData.getSum(),
convertAttributes(scope, histogramData.getAttributes()),
convertDoubleExemplars(histogramData.getExemplars()),
histogramData.getStartEpochNanos() / NANOS_PER_MILLISECOND);
histogramData.getStartEpochNanos() / NANOS_PER_MILLISECOND));
}
return new HistogramSnapshot(metadata, Arrays.asList(data));
return new HistogramSnapshot(metadata, data);
}

@Nullable
private HistogramSnapshot convertExponentialHistogram(
MetricMetadata metadata,
InstrumentationScopeInfo scope,
Collection<ExponentialHistogramPointData> dataPoints) {
HistogramDataPointSnapshot[] data = new HistogramDataPointSnapshot[dataPoints.size()];
int i = 0;
List<HistogramDataPointSnapshot> data = new ArrayList<>(dataPoints.size());
for (ExponentialHistogramPointData histogramData : dataPoints) {
int scale = histogramData.getScale();
if (scale < -4) {
// Scale < -4 is not supported in Prometheus. Histograms with scale < -4 are dropped.
THROTTLING_LOGGER.log(

Check warning on line 259 in exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java

View check run for this annotation

Codecov / codecov/patch

exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java#L259

Added line #L259 was not covered by tests
Level.WARNING,
"Dropping histogram "
+ metadata.getName()

Check warning on line 262 in exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java

View check run for this annotation

Codecov / codecov/patch

exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java#L262

Added line #L262 was not covered by tests
+ " with attributes "
+ histogramData.getAttributes()

Check warning on line 264 in exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java

View check run for this annotation

Codecov / codecov/patch

exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java#L264

Added line #L264 was not covered by tests
+ " because it has scale < -4 which is unsupported in Prometheus");
return null;

Check warning on line 266 in exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java

View check run for this annotation

Codecov / codecov/patch

exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java#L266

Added line #L266 was not covered by tests
}
// Scale > 8 are not supported in Prometheus. Histograms with scale > 8 are scaled down to 8.
int scaleDown = scale > 8 ? scale - 8 : 0;
data[i++] =
data.add(
new HistogramDataPointSnapshot(
scale - scaleDown,
histogramData.getZeroCount(),
Expand All @@ -273,13 +277,12 @@ private HistogramSnapshot convertExponentialHistogram(
histogramData.getSum(),
convertAttributes(scope, histogramData.getAttributes()),
convertDoubleExemplars(histogramData.getExemplars()),
histogramData.getStartEpochNanos() / NANOS_PER_MILLISECOND);
histogramData.getStartEpochNanos() / NANOS_PER_MILLISECOND));
}
return new HistogramSnapshot(metadata, Arrays.asList(data));
return new HistogramSnapshot(metadata, data);
}

@SuppressWarnings("MethodCanBeStatic")
private NativeHistogramBuckets convertExponentialHistogramBuckets(
private static NativeHistogramBuckets convertExponentialHistogramBuckets(
ExponentialHistogramBuckets buckets, int scaleDown) {
if (buckets.getBucketCounts().isEmpty()) {
return NativeHistogramBuckets.EMPTY;
Expand Down Expand Up @@ -308,55 +311,54 @@ private SummarySnapshot convertSummary(
MetricMetadata metadata,
InstrumentationScopeInfo scope,
Collection<SummaryPointData> dataPoints) {
SummaryDataPointSnapshot[] data = new SummaryDataPointSnapshot[dataPoints.size()];
int i = 0;
List<SummaryDataPointSnapshot> data = new ArrayList<>(dataPoints.size());

Check warning on line 314 in exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java

View check run for this annotation

Codecov / codecov/patch

exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java#L314

Added line #L314 was not covered by tests
for (SummaryPointData summaryData : dataPoints) {
data[i++] =
data.add(

Check warning on line 316 in exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java

View check run for this annotation

Codecov / codecov/patch

exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java#L316

Added line #L316 was not covered by tests
new SummaryDataPointSnapshot(
summaryData.getCount(),
summaryData.getSum(),
convertQuantiles(summaryData.getValues()),
convertAttributes(scope, summaryData.getAttributes()),

Check warning on line 321 in exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java

View check run for this annotation

Codecov / codecov/patch

exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java#L318-L321

Added lines #L318 - L321 were not covered by tests
Exemplars.EMPTY, // Exemplars for Summaries not implemented yet.
summaryData.getStartEpochNanos() / NANOS_PER_MILLISECOND);
summaryData.getStartEpochNanos() / NANOS_PER_MILLISECOND));
}
return new SummarySnapshot(metadata, Arrays.asList(data));
return new SummarySnapshot(metadata, data);

Check warning on line 325 in exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java

View check run for this annotation

Codecov / codecov/patch

exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java#L323-L325

Added lines #L323 - L325 were not covered by tests
}

@SuppressWarnings("MethodCanBeStatic")
private Quantiles convertQuantiles(List<ValueAtQuantile> values) {
Quantile[] result = new Quantile[values.size()];
for (int i = 0; i < result.length; i++) {
result[i] = new Quantile(values.get(i).getQuantile(), values.get(i).getValue());
private static Quantiles convertQuantiles(List<ValueAtQuantile> values) {
List<Quantile> result = new ArrayList<>(values.size());

Check warning on line 329 in exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java

View check run for this annotation

Codecov / codecov/patch

exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java#L329

Added line #L329 was not covered by tests
for (ValueAtQuantile value : values) {
result.add(new Quantile(value.getQuantile(), value.getValue()));
}
return Quantiles.of(result);

Check warning on line 333 in exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java

View check run for this annotation

Codecov / codecov/patch

exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java#L331-L333

Added lines #L331 - L333 were not covered by tests
}

@Nullable
private Exemplar convertLongExemplar(List<LongExemplarData> exemplars) {
if (exemplars.size() == 0) {
if (exemplars.isEmpty()) {
return null;
} else {
LongExemplarData exemplar = exemplars.get(0);
return convertExemplar((double) exemplar.getValue(), exemplar);
}
}

/** Converts the first exemplar in the list if available, else returns {#code null}. */
@Nullable
private Exemplar convertDoubleExemplar(List<DoubleExemplarData> exemplars) {
if (exemplars.size() == 0) {
if (exemplars.isEmpty()) {
return null;
} else {
DoubleExemplarData exemplar = exemplars.get(0);
return convertExemplar(exemplar.getValue(), exemplar);
}
}

/** Converts the first exemplar in the list if available, else returns {#code null}. */
private Exemplars convertDoubleExemplars(List<DoubleExemplarData> exemplars) {
Exemplar[] result = new Exemplar[exemplars.size()];
for (int i = 0; i < result.length; i++) {
DoubleExemplarData exemplar = exemplars.get(i);
result[i] = convertExemplar(exemplar.getValue(), exemplar);
List<Exemplar> result = new ArrayList<>(exemplars.size());
for (DoubleExemplarData exemplar : exemplars) {
result.add(convertExemplar(exemplar.getValue(), exemplar));
}
return Exemplars.of(result);
}
Expand Down Expand Up @@ -398,6 +400,14 @@ private InfoSnapshot makeScopeInfo(Set<InstrumentationScopeInfo> scopes) {
return new InfoSnapshot(new MetricMetadata("otel_scope"), prometheusScopeInfos);

Check warning on line 400 in exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java

View check run for this annotation

Codecov / codecov/patch

exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java#L397-L400

Added lines #L397 - L400 were not covered by tests
}

/**
* Convert OpenTelemetry attributes to Prometheus labels.
*
* @param scope will be converted to {@code otel_scope_*} labels if {@code otelScopeEnabled} is
* {@code true}.
* @param attributes the attributes to be converted.
* @param additionalAttributes optional list of key/value pairs, may be empty.
*/
private Labels convertAttributes(
@Nullable InstrumentationScopeInfo scope,
Attributes attributes,
Expand Down Expand Up @@ -434,8 +444,7 @@ private Labels convertAttributes(
return Labels.of(names, values);
}

@SuppressWarnings("MethodCanBeStatic")
private MetricMetadata convertMetadata(MetricData metricData) {
private static MetricMetadata convertMetadata(MetricData metricData) {
String name = sanitizeMetricName(metricData.getName());
String help = metricData.getDescription();
Unit unit = PrometheusUnitsHelper.convertUnit(metricData.getUnit());
Expand All @@ -445,7 +454,8 @@ private MetricMetadata convertMetadata(MetricData metricData) {
return new MetricMetadata(name, help, unit);
}

private void putOrMerge(Map<String, MetricSnapshot> snapshotsByName, MetricSnapshot snapshot) {
private static void putOrMerge(
Map<String, MetricSnapshot> snapshotsByName, MetricSnapshot snapshot) {
String name = snapshot.getMetadata().getName();
if (snapshotsByName.containsKey(name)) {
MetricSnapshot merged = merge(snapshotsByName.get(name), snapshot);
Expand All @@ -464,7 +474,7 @@ private void putOrMerge(Map<String, MetricSnapshot> snapshotsByName, MetricSnaps
* type. If the type differs, we log a message and drop one of them.
*/
@Nullable
private MetricSnapshot merge(MetricSnapshot a, MetricSnapshot b) {
private static MetricSnapshot merge(MetricSnapshot a, MetricSnapshot b) {
MetricMetadata metadata = mergeMetadata(a.getMetadata(), b.getMetadata());
if (metadata == null) {
return null;

Check warning on line 480 in exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java

View check run for this annotation

Codecov / codecov/patch

exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java#L480

Added line #L480 was not covered by tests
Expand Down Expand Up @@ -512,8 +522,7 @@ private MetricSnapshot merge(MetricSnapshot a, MetricSnapshot b) {
}

@Nullable
@SuppressWarnings("MethodCanBeStatic")
private MetricMetadata mergeMetadata(MetricMetadata a, MetricMetadata b) {
private static MetricMetadata mergeMetadata(MetricMetadata a, MetricMetadata b) {
String name = a.getPrometheusName();
if (a.getName().equals(b.getName())) {
name = a.getName();
Expand All @@ -535,8 +544,7 @@ private MetricMetadata mergeMetadata(MetricMetadata a, MetricMetadata b) {
return new MetricMetadata(name, help, unit);
}

@SuppressWarnings("MethodCanBeStatic")
private String typeString(MetricSnapshot snapshot) {
private static String typeString(MetricSnapshot snapshot) {
// Simple helper for a log message.
return snapshot.getClass().getSimpleName().replace("Snapshot", "").toLowerCase(Locale.ENGLISH);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public static PrometheusHttpServerBuilder builder() {
.hostname(host)
.port(port)
.executorService(executor)
.registry(prometheusRegistry)
.defaultHandler(new MetricsHandler())
.buildAndStart();
} catch (IOException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public final class PrometheusHttpServerBuilder {

private String host = DEFAULT_HOST;
private int port = DEFAULT_PORT;
private PrometheusRegistry prometheusRegistry = PrometheusRegistry.defaultRegistry;
private PrometheusRegistry prometheusRegistry = new PrometheusRegistry();
private boolean otelScopeEnabled = true;

@Nullable private ExecutorService executor;
Expand Down
Loading

0 comments on commit 79717ba

Please sign in to comment.