Skip to content

Commit e9aeabe

Browse files
ghareeb-falazighareeb-falazi
committed
apply review comments
1 parent 62b4f02 commit e9aeabe

File tree

5 files changed

+20
-26
lines changed

5 files changed

+20
-26
lines changed

‎docs/supported-libraries.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ These are the supported libraries and frameworks:
3131
| [Apache Dubbo](https://github.com/apache/dubbo/) | 2.7+ | [opentelemetry-apache-dubbo-2.7](../instrumentation/apache-dubbo-2.7/library-autoconfigure) | [RPC Client Spans], [RPC Server Spans] |
3232
| [Apache HttpAsyncClient](https://hc.apache.org/index.html) | 4.1+ | N/A | [HTTP Client Spans], [HTTP Client Metrics] |
3333
| [Apache HttpClient](https://hc.apache.org/index.html) | 2.0+ | [opentelemetry-apache-httpclient-4.3](../instrumentation/apache-httpclient/apache-httpclient-4.3/library),<br>[opentelemetry-apache-httpclient-5.2](../instrumentation/apache-httpclient/apache-httpclient-5.2/library) | [HTTP Client Spans], [HTTP Client Metrics] |
34+
| [Apache Iceberg](https://iceberg.apache.org/) | N/A | [opentelemetry-iceberg-1.8](../instrumentation/iceberg-1.8/library/) | none |
3435
| [Apache ShenYu](https://shenyu.apache.org/) | 2.4+ | N/A | Provides `http.route` [2] |
3536
| [Apache Kafka Connect API](https://kafka.apache.org/documentation/#connect) | 2.6+ | N/A | [Messaging Spans] |
3637
| [Apache Kafka Producer/Consumer API](https://kafka.apache.org/documentation/#producerapi) | 0.11+ | [opentelemetry-kafka-clients-2.6](../instrumentation/kafka/kafka-clients/kafka-clients-2.6/library) | [Messaging Spans] |

‎instrumentation/iceberg-1.8/library/src/main/java/io/opentelemetry/instrumentation/iceberg/v1_8/IcebergMetricsReporter.java‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,16 @@
88
import io.opentelemetry.api.OpenTelemetry;
99
import io.opentelemetry.api.common.AttributeKey;
1010
import io.opentelemetry.api.common.Attributes;
11+
import io.opentelemetry.api.metrics.DoubleHistogram;
1112
import io.opentelemetry.api.metrics.LongCounter;
12-
import io.opentelemetry.api.metrics.LongGauge;
13-
import java.util.Locale;
1413
import org.apache.iceberg.metrics.CounterResult;
1514
import org.apache.iceberg.metrics.MetricsReport;
1615
import org.apache.iceberg.metrics.MetricsReporter;
1716
import org.apache.iceberg.metrics.ScanMetricsResult;
1817
import org.apache.iceberg.metrics.ScanReport;
1918
import org.apache.iceberg.metrics.TimerResult;
2019

21-
public class IcebergMetricsReporter implements MetricsReporter {
20+
final class IcebergMetricsReporter implements MetricsReporter {
2221
private static final String INSTRUMENTATION_NAME = "io.opentelemetry.iceberg-1.8";
2322
private static final AttributeKey<Long> SCHEMA_ID = AttributeKey.longKey("iceberg.schema.id");
2423
private static final AttributeKey<String> TABLE_NAME =
@@ -51,11 +50,10 @@ void reportScanMetrics(ScanReport scanReport) {
5150
TimerResult duration = metrics.totalPlanningDuration();
5251

5352
if (duration != null) {
54-
LongGauge metric =
53+
DoubleHistogram metric =
5554
ScanMetricsBuilder.totalPlanningDuration(
56-
openTelemetry.getMeter(INSTRUMENTATION_NAME),
57-
duration.timeUnit().name().toLowerCase(Locale.getDefault()));
58-
metric.set(duration.totalDuration().toMillis(), scanAttributes);
55+
openTelemetry.getMeter(INSTRUMENTATION_NAME), "s");
56+
metric.record(duration.totalDuration().toMillis() / 1000.0, scanAttributes);
5957
}
6058

6159
CounterResult current = metrics.resultDataFiles();

‎instrumentation/iceberg-1.8/library/src/main/java/io/opentelemetry/instrumentation/iceberg/v1_8/IcebergTelemetry.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
import org.apache.iceberg.ScanTask;
1111
import org.apache.iceberg.ScanTaskGroup;
1212

13-
public class IcebergTelemetry {
13+
public final class IcebergTelemetry {
1414
private final OpenTelemetry openTelemetry;
1515

1616
public static IcebergTelemetry create(OpenTelemetry openTelemetry) {

‎instrumentation/iceberg-1.8/library/src/main/java/io/opentelemetry/instrumentation/iceberg/v1_8/ScanMetricsBuilder.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55

66
package io.opentelemetry.instrumentation.iceberg.v1_8;
77

8+
import io.opentelemetry.api.metrics.DoubleHistogram;
89
import io.opentelemetry.api.metrics.LongCounter;
9-
import io.opentelemetry.api.metrics.LongGauge;
1010
import io.opentelemetry.api.metrics.Meter;
1111

1212
final class ScanMetricsBuilder {
@@ -34,12 +34,11 @@ private ScanMetricsBuilder() {
3434
// prevents instantiation
3535
}
3636

37-
static LongGauge totalPlanningDuration(Meter meter, String unit) {
37+
static DoubleHistogram totalPlanningDuration(Meter meter, String unit) {
3838
return meter
39-
.gaugeBuilder(TOTAL_PLANNING_DURATION)
39+
.histogramBuilder(TOTAL_PLANNING_DURATION)
4040
.setDescription("The total duration needed to plan the scan.")
4141
.setUnit(unit)
42-
.ofLongs()
4342
.build();
4443
}
4544

‎instrumentation/iceberg-1.8/testing/src/main/java/io/opentelemetry/instrumentation/iceberg/v1_8/AbstractIcebergTest.java‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
import java.io.File;
1515
import java.io.IOException;
1616
import java.util.Arrays;
17-
import java.util.Locale;
1817
import org.apache.iceberg.DataFile;
1918
import org.apache.iceberg.DataFiles;
2019
import org.apache.iceberg.FileScanTask;
@@ -282,11 +281,7 @@ void testCreateTelemetry() throws IOException {
282281
TimerResult timer = expected.scanMetrics().totalPlanningDuration();
283282

284283
if (timer != null) {
285-
assertIcebergGaugeMetric(
286-
"iceberg.scan.planning.duration",
287-
timer.timeUnit().name().toLowerCase(Locale.getDefault()),
288-
expected,
289-
timer.totalDuration().toMillis());
284+
assertIcebergHistogramMetric("iceberg.scan.planning.duration", "s", expected);
290285
} else {
291286
assertIcebergMetricNotReported("iceberg.scan.planning.duration");
292287
}
@@ -303,21 +298,22 @@ private void assertIcebergMetricNotReported(String otelMetricName) {
303298
"metric is not reported")));
304299
}
305300

306-
private void assertIcebergGaugeMetric(
307-
String otelMetricName, String expectedUnit, ScanReport expectedReport, long expectedValue) {
301+
private void assertIcebergHistogramMetric(
302+
String otelMetricName, String expectedUnit, ScanReport expectedReport) {
308303
testing()
309304
.waitAndAssertMetrics(
310305
"io.opentelemetry.iceberg-1.8",
311306
metricAssert ->
312307
metricAssert
313308
.hasName(otelMetricName)
314309
.hasUnit(expectedUnit)
315-
.hasLongGaugeSatisfying(
316-
sum ->
317-
sum.hasPointsSatisfying(
318-
longAssert ->
319-
longAssert
320-
.hasValue(expectedValue)
310+
.hasHistogramSatisfying(
311+
histogram ->
312+
histogram.hasPointsSatisfying(
313+
point ->
314+
point
315+
.hasSumGreaterThan(0.0)
316+
.hasCount(1)
321317
.hasAttributesSatisfying(
322318
attributes ->
323319
assertEquals(

0 commit comments

Comments
 (0)