Skip to content

Commit

Permalink
Migrate Metrics SDK "data' package to Attributes (#3352)
Browse files Browse the repository at this point in the history 
* Update java metrics data model and create assertion based testing library.

* Update existing SDK for Label => Attribute in data model

- Finish wiring through changes to all code/tests.
- DOES NOT touch API
- Update existing tests to use new testing library to be sensitive to attribute vs. label hashing alterations + equality of the metrics.data package types.

* Fix javadoc breakages

* Fix JMH compile

* Finish javadoc

* Fixes from review

* Remove botched file.

* Fixes from review.
  • Loading branch information
@jsuereth
jsuereth authored Jul 1, 2021
1 parent 0e0d9b5 commit b12a57e
Show file tree
Hide file tree
Showing 74 changed files with 2,623 additions and 1,613 deletions.
8 changes: 5 additions & 3 deletions ...c/test/java/io/opentelemetry/exporter/logging/otlp/OtlpJsonLoggingMetricExporterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@

package io.opentelemetry.exporter.logging.otlp;

import static io.opentelemetry.api.common.AttributeKey.stringKey;
import static org.assertj.core.api.Assertions.assertThat;

import io.github.netmikey.logunit.api.LogCapturer;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.metrics.common.Labels;
import io.opentelemetry.sdk.common.InstrumentationLibraryInfo;
import io.opentelemetry.sdk.metrics.data.AggregationTemporality;
import io.opentelemetry.sdk.metrics.data.DoublePointData;
Expand Down Expand Up @@ -39,7 +39,8 @@ class OtlpJsonLoggingMetricExporterTest {
DoubleSumData.create(
true,
AggregationTemporality.CUMULATIVE,
Arrays.asList(DoublePointData.create(1, 2, Labels.of("cat", "meow"), 4))));
Arrays.asList(
DoublePointData.create(1, 2, Attributes.of(stringKey("cat"), "meow"), 4))));

private static final MetricData METRIC2 =
MetricData.createDoubleSum(
Expand All @@ -51,7 +52,8 @@ class OtlpJsonLoggingMetricExporterTest {
DoubleSumData.create(
true,
AggregationTemporality.CUMULATIVE,
Arrays.asList(DoublePointData.create(1, 2, Labels.of("cat", "meow"), 4))));
Arrays.asList(
DoublePointData.create(1, 2, Attributes.of(stringKey("cat"), "meow"), 4))));

@RegisterExtension
LogCapturer logs = LogCapturer.create().captureForType(OtlpJsonLoggingMetricExporter.class);
Expand Down
7 changes: 3 additions & 4 deletions ...rs/logging/src/test/java/io/opentelemetry/exporter/logging/LoggingMetricExporterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.metrics.common.Labels;
import io.opentelemetry.sdk.common.InstrumentationLibraryInfo;
import io.opentelemetry.sdk.metrics.data.AggregationTemporality;
import io.opentelemetry.sdk.metrics.data.DoublePointData;
Expand Down Expand Up @@ -67,7 +66,7 @@ void testExport() {
DoubleSummaryPointData.create(
nowEpochNanos,
nowEpochNanos + 245,
Labels.of("a", "b", "c", "d"),
Attributes.of(stringKey("a"), "b", stringKey("c"), "d"),
1010,
50000,
Arrays.asList(
Expand All @@ -86,7 +85,7 @@ void testExport() {
LongPointData.create(
nowEpochNanos,
nowEpochNanos + 245,
Labels.of("z", "y", "x", "w"),
Attributes.of(stringKey("z"), "y", stringKey("x"), "w"),
1010)))),
MetricData.createDoubleSum(
resource,
Expand All @@ -101,7 +100,7 @@ void testExport() {
DoublePointData.create(
nowEpochNanos,
nowEpochNanos + 245,
Labels.of("1", "2", "3", "4"),
Attributes.of(stringKey("1"), "2", stringKey("3"), "4"),
33.7767))))));
}

Expand Down
52 changes: 16 additions & 36 deletions ...ters/otlp/common/src/main/java/io/opentelemetry/exporter/otlp/internal/MetricAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@
import static io.opentelemetry.proto.metrics.v1.AggregationTemporality.AGGREGATION_TEMPORALITY_DELTA;
import static io.opentelemetry.proto.metrics.v1.AggregationTemporality.AGGREGATION_TEMPORALITY_UNSPECIFIED;

import io.opentelemetry.api.metrics.common.Labels;
import io.opentelemetry.proto.common.v1.AnyValue;
import io.opentelemetry.proto.common.v1.KeyValue;
import io.opentelemetry.proto.metrics.v1.AggregationTemporality;
import io.opentelemetry.proto.metrics.v1.Gauge;
import io.opentelemetry.proto.metrics.v1.Histogram;
Expand Down Expand Up @@ -39,7 +36,6 @@
import io.opentelemetry.sdk.resources.Resource;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -199,10 +195,10 @@ static List<NumberDataPoint> toIntDataPoints(Collection<LongPointData> points) {
.setStartTimeUnixNano(longPoint.getStartEpochNanos())
.setTimeUnixNano(longPoint.getEpochNanos())
.setAsInt(longPoint.getValue());
Collection<KeyValue> labels = toProtoLabels(longPoint.getLabels());
if (!labels.isEmpty()) {
builder.addAllAttributes(labels);
}
longPoint
.getAttributes()
.forEach(
(key, value) -> builder.addAttributes(CommonAdapter.toProtoAttribute(key, value)));
result.add(builder.build());
}
return result;
Expand All @@ -216,10 +212,10 @@ static Collection<NumberDataPoint> toDoubleDataPoints(Collection<DoublePointData
.setStartTimeUnixNano(doublePoint.getStartEpochNanos())
.setTimeUnixNano(doublePoint.getEpochNanos())
.setAsDouble(doublePoint.getValue());
Collection<KeyValue> labels = toProtoLabels(doublePoint.getLabels());
if (!labels.isEmpty()) {
builder.addAllAttributes(labels);
}
doublePoint
.getAttributes()
.forEach(
(key, value) -> builder.addAttributes(CommonAdapter.toProtoAttribute(key, value)));
result.add(builder.build());
}
return result;
Expand All @@ -234,10 +230,10 @@ static List<SummaryDataPoint> toSummaryDataPoints(Collection<DoubleSummaryPointD
.setTimeUnixNano(doubleSummaryPoint.getEpochNanos())
.setCount(doubleSummaryPoint.getCount())
.setSum(doubleSummaryPoint.getSum());
List<KeyValue> labels = toProtoLabels(doubleSummaryPoint.getLabels());
if (!labels.isEmpty()) {
builder.addAllAttributes(labels);
}
doubleSummaryPoint
.getAttributes()
.forEach(
(key, value) -> builder.addAttributes(CommonAdapter.toProtoAttribute(key, value)));
// Not calling directly addAllQuantileValues because that generates couple of unnecessary
// allocations if empty list.
if (!doubleSummaryPoint.getPercentileValues().isEmpty()) {
Expand Down Expand Up @@ -269,30 +265,14 @@ static Collection<HistogramDataPoint> toHistogramDataPoints(
if (!boundaries.isEmpty()) {
builder.addAllExplicitBounds(boundaries);
}
Collection<KeyValue> labels = toProtoLabels(doubleHistogramPoint.getLabels());
if (!labels.isEmpty()) {
builder.addAllAttributes(labels);
}
doubleHistogramPoint
.getAttributes()
.forEach(
(key, value) -> builder.addAttributes(CommonAdapter.toProtoAttribute(key, value)));
result.add(builder.build());
}
return result;
}

@SuppressWarnings("MixedMutabilityReturnType")
static List<KeyValue> toProtoLabels(Labels labels) {
if (labels.isEmpty()) {
return Collections.emptyList();
}
final List<KeyValue> result = new ArrayList<>(labels.size());
labels.forEach(
(key, value) ->
result.add(
KeyValue.newBuilder()
.setKey(key)
.setValue(AnyValue.newBuilder().setStringValue(value).build())
.build()));
return result;
}

private MetricAdapter() {}
}
70 changes: 29 additions & 41 deletions .../otlp/common/src/test/java/io/opentelemetry/exporter/otlp/internal/MetricAdapterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

import com.google.common.collect.ImmutableList;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.metrics.common.Labels;
import io.opentelemetry.proto.common.v1.AnyValue;
import io.opentelemetry.proto.common.v1.InstrumentationLibrary;
import io.opentelemetry.proto.common.v1.KeyValue;
Expand Down Expand Up @@ -46,16 +45,8 @@
import org.junit.jupiter.api.Test;

class MetricAdapterTest {
@Test
void toProtoLabels() {
assertThat(MetricAdapter.toProtoLabels(Labels.empty())).isEmpty();
assertThat(MetricAdapter.toProtoLabels(Labels.of("k", "v")))
.containsExactly(KeyValue.newBuilder().setKey("k").setValue(stringValue("v")).build());
assertThat(MetricAdapter.toProtoLabels(Labels.of("k1", "v1", "k2", "v2")))
.containsExactly(
KeyValue.newBuilder().setKey("k1").setValue(stringValue("v1")).build(),
KeyValue.newBuilder().setKey("k2").setValue(stringValue("v2")).build());
}

private static final Attributes KV_ATTR = Attributes.of(stringKey("k"), "v");

private static AnyValue stringValue(String v) {
return AnyValue.newBuilder().setStringValue(v).build();
Expand All @@ -66,7 +57,7 @@ void toInt64DataPoints() {
assertThat(MetricAdapter.toIntDataPoints(Collections.emptyList())).isEmpty();
assertThat(
MetricAdapter.toIntDataPoints(
singletonList(LongPointData.create(123, 456, Labels.of("k", "v"), 5))))
singletonList(LongPointData.create(123, 456, KV_ATTR, 5))))
.containsExactly(
NumberDataPoint.newBuilder()
.setStartTimeUnixNano(123)
Expand All @@ -79,8 +70,8 @@ void toInt64DataPoints() {
assertThat(
MetricAdapter.toIntDataPoints(
ImmutableList.of(
LongPointData.create(123, 456, Labels.empty(), 5),
LongPointData.create(321, 654, Labels.of("k", "v"), 7))))
LongPointData.create(123, 456, Attributes.empty(), 5),
LongPointData.create(321, 654, KV_ATTR, 7))))
.containsExactly(
NumberDataPoint.newBuilder()
.setStartTimeUnixNano(123)
Expand All @@ -102,7 +93,7 @@ void toDoubleDataPoints() {
assertThat(MetricAdapter.toDoubleDataPoints(Collections.emptyList())).isEmpty();
assertThat(
MetricAdapter.toDoubleDataPoints(
singletonList(DoublePointData.create(123, 456, Labels.of("k", "v"), 5.1))))
singletonList(DoublePointData.create(123, 456, KV_ATTR, 5.1))))
.containsExactly(
NumberDataPoint.newBuilder()
.setStartTimeUnixNano(123)
Expand All @@ -115,8 +106,8 @@ void toDoubleDataPoints() {
assertThat(
MetricAdapter.toDoubleDataPoints(
ImmutableList.of(
DoublePointData.create(123, 456, Labels.empty(), 5.1),
DoublePointData.create(321, 654, Labels.of("k", "v"), 7.1))))
DoublePointData.create(123, 456, Attributes.empty(), 5.1),
DoublePointData.create(321, 654, KV_ATTR, 7.1))))
.containsExactly(
NumberDataPoint.newBuilder()
.setStartTimeUnixNano(123)
Expand All @@ -141,7 +132,7 @@ void toSummaryDataPoints() {
DoubleSummaryPointData.create(
123,
456,
Labels.of("k", "v"),
KV_ATTR,
5,
14.2,
singletonList(ValueAtPercentile.create(0.0, 1.1))))))
Expand All @@ -164,11 +155,11 @@ void toSummaryDataPoints() {
MetricAdapter.toSummaryDataPoints(
ImmutableList.of(
DoubleSummaryPointData.create(
123, 456, Labels.empty(), 7, 15.3, Collections.emptyList()),
123, 456, Attributes.empty(), 7, 15.3, Collections.emptyList()),
DoubleSummaryPointData.create(
321,
654,
Labels.of("k", "v"),
KV_ATTR,
9,
18.3,
ImmutableList.of(
Expand Down Expand Up @@ -207,15 +198,15 @@ void toHistogramDataPoints() {
assertThat(
MetricAdapter.toHistogramDataPoints(
ImmutableList.of(
DoubleHistogramPointData.create(
123, 456, KV_ATTR, 14.2, ImmutableList.of(1.0), ImmutableList.of(1L, 5L)),
DoubleHistogramPointData.create(
123,
456,
Labels.of("k", "v"),
14.2,
ImmutableList.of(1.0),
ImmutableList.of(1L, 5L)),
DoubleHistogramPointData.create(
123, 456, Labels.empty(), 15.3, ImmutableList.of(), ImmutableList.of(7L)))))
Attributes.empty(),
15.3,
ImmutableList.of(),
ImmutableList.of(7L)))))
.containsExactly(
HistogramDataPoint.newBuilder()
.setStartTimeUnixNano(123)
Expand Down Expand Up @@ -251,7 +242,7 @@ void toProtoMetric_monotonic() {
LongSumData.create(
/* isMonotonic= */ true,
AggregationTemporality.CUMULATIVE,
singletonList(LongPointData.create(123, 456, Labels.of("k", "v"), 5))))))
singletonList(LongPointData.create(123, 456, KV_ATTR, 5))))))
.isEqualTo(
Metric.newBuilder()
.setName("name")
Expand Down Expand Up @@ -286,8 +277,7 @@ void toProtoMetric_monotonic() {
DoubleSumData.create(
/* isMonotonic= */ true,
AggregationTemporality.CUMULATIVE,
singletonList(
DoublePointData.create(123, 456, Labels.of("k", "v"), 5.1))))))
singletonList(DoublePointData.create(123, 456, KV_ATTR, 5.1))))))
.isEqualTo(
Metric.newBuilder()
.setName("name")
Expand Down Expand Up @@ -326,7 +316,7 @@ void toProtoMetric_nonMonotonic() {
LongSumData.create(
/* isMonotonic= */ false,
AggregationTemporality.CUMULATIVE,
singletonList(LongPointData.create(123, 456, Labels.of("k", "v"), 5))))))
singletonList(LongPointData.create(123, 456, KV_ATTR, 5))))))
.isEqualTo(
Metric.newBuilder()
.setName("name")
Expand Down Expand Up @@ -361,8 +351,7 @@ void toProtoMetric_nonMonotonic() {
DoubleSumData.create(
/* isMonotonic= */ false,
AggregationTemporality.CUMULATIVE,
singletonList(
DoublePointData.create(123, 456, Labels.of("k", "v"), 5.1))))))
singletonList(DoublePointData.create(123, 456, KV_ATTR, 5.1))))))
.isEqualTo(
Metric.newBuilder()
.setName("name")
Expand Down Expand Up @@ -399,7 +388,7 @@ void toProtoMetric_gauges() {
"description",
"1",
LongGaugeData.create(
singletonList(LongPointData.create(123, 456, Labels.of("k", "v"), 5))))))
singletonList(LongPointData.create(123, 456, KV_ATTR, 5))))))
.isEqualTo(
Metric.newBuilder()
.setName("name")
Expand Down Expand Up @@ -430,8 +419,7 @@ void toProtoMetric_gauges() {
"description",
"1",
DoubleGaugeData.create(
singletonList(
DoublePointData.create(123, 456, Labels.of("k", "v"), 5.1))))))
singletonList(DoublePointData.create(123, 456, KV_ATTR, 5.1))))))
.isEqualTo(
Metric.newBuilder()
.setName("name")
Expand Down Expand Up @@ -470,7 +458,7 @@ void toProtoMetric_summary() {
DoubleSummaryPointData.create(
123,
456,
Labels.of("k", "v"),
KV_ATTR,
5,
33d,
ImmutableList.of(
Expand Down Expand Up @@ -526,7 +514,7 @@ void toProtoMetric_histogram() {
DoubleHistogramPointData.create(
123,
456,
Labels.of("k", "v"),
KV_ATTR,
4.0,
ImmutableList.of(),
ImmutableList.of(33L)))))))
Expand Down Expand Up @@ -611,7 +599,7 @@ void toProtoResourceMetrics() {
/* isMonotonic= */ true,
AggregationTemporality.CUMULATIVE,
Collections.singletonList(
DoublePointData.create(123, 456, Labels.of("k", "v"), 5.0)))),
DoublePointData.create(123, 456, KV_ATTR, 5.0)))),
MetricData.createDoubleSum(
resource,
instrumentationLibraryInfo,
Expand All @@ -622,7 +610,7 @@ void toProtoResourceMetrics() {
/* isMonotonic= */ true,
AggregationTemporality.CUMULATIVE,
Collections.singletonList(
DoublePointData.create(123, 456, Labels.of("k", "v"), 5.0)))),
DoublePointData.create(123, 456, KV_ATTR, 5.0)))),
MetricData.createDoubleSum(
Resource.empty(),
instrumentationLibraryInfo,
Expand All @@ -633,7 +621,7 @@ void toProtoResourceMetrics() {
/* isMonotonic= */ true,
AggregationTemporality.CUMULATIVE,
Collections.singletonList(
DoublePointData.create(123, 456, Labels.of("k", "v"), 5.0)))),
DoublePointData.create(123, 456, KV_ATTR, 5.0)))),
MetricData.createDoubleSum(
Resource.empty(),
InstrumentationLibraryInfo.empty(),
Expand All @@ -644,7 +632,7 @@ void toProtoResourceMetrics() {
/* isMonotonic= */ true,
AggregationTemporality.CUMULATIVE,
Collections.singletonList(
DoublePointData.create(123, 456, Labels.of("k", "v"), 5.0)))))))
DoublePointData.create(123, 456, KV_ATTR, 5.0)))))))
.containsExactlyInAnyOrder(
ResourceMetrics.newBuilder()
.setResource(resourceProto)
Expand Down
Loading

0 comments on commit b12a57e

Please sign in to comment.