From 00d7ade0eecc15606b7627609d154818073dcd72 Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Wed, 13 Sep 2023 14:39:42 -0500 Subject: [PATCH 1/3] ConfigurationReader handles null values as empty --- .../kotlin/otel.java-conventions.gradle.kts | 5 +- sdk-extensions/incubator/build.gradle.kts | 4 +- .../fileconfig/ConfigurationReader.java | 8 ++- .../fileconfig/AggregationFactoryTest.java | 9 ++- .../fileconfig/ConfigurationReaderTest.java | 57 +++++++++++++++++++ 5 files changed, 73 insertions(+), 10 deletions(-) diff --git a/buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts b/buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts index bf63d7db4c5..b4df8594f02 100644 --- a/buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts +++ b/buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts @@ -74,10 +74,7 @@ tasks { // https://groups.google.com/forum/#!topic/bazel-discuss/_R3A9TJSoPM "-Xlint:-processing", // We suppress the "options" warning because it prevents compilation on modern JDKs - "-Xlint:-options", - - // Fail build on any warning - "-Werror", + "-Xlint:-options" ), ) } diff --git a/sdk-extensions/incubator/build.gradle.kts b/sdk-extensions/incubator/build.gradle.kts index 35a58968fa8..3dffd250461 100644 --- a/sdk-extensions/incubator/build.gradle.kts +++ b/sdk-extensions/incubator/build.gradle.kts @@ -55,8 +55,8 @@ dependencies { // ... proceed with normal sourcesJar, compileJava, etc // TODO(jack-berg): update ref to be released version when available -val configurationRef = "0eb96de17c6533f668163873d95bd026bce1d8fb" -val configurationRepoZip = "https://github.com/open-telemetry/opentelemetry-configuration/archive/$configurationRef.zip" +val configurationRef = "ad219187cdb03c8d8ea1a2311afd280bf64d2262" +val configurationRepoZip = "https://github.com/jack-berg/opentelemetry-configuration/archive/$configurationRef.zip" val buildDirectory = layout.buildDirectory.asFile.get() val downloadConfigurationSchema by tasks.registering(Download::class) { diff --git a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ConfigurationReader.java b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ConfigurationReader.java index 04aaaa894ef..fab876a8447 100644 --- a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ConfigurationReader.java +++ b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ConfigurationReader.java @@ -5,7 +5,10 @@ package io.opentelemetry.sdk.extension.incubator.fileconfig; +import com.fasterxml.jackson.annotation.JsonSetter; +import com.fasterxml.jackson.annotation.Nulls; import com.fasterxml.jackson.databind.ObjectMapper; +import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.Aggregation; import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.OpenTelemetryConfiguration; import java.io.InputStream; import org.snakeyaml.engine.v2.api.Load; @@ -13,7 +16,10 @@ final class ConfigurationReader { - private static final ObjectMapper MAPPER = new ObjectMapper(); + private static final ObjectMapper MAPPER = + new ObjectMapper() + // Create empty object instances for keys which are present but have null values + .setDefaultSetterInfo(JsonSetter.Value.forValueNulls(Nulls.AS_EMPTY)); private ConfigurationReader() {} diff --git a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/AggregationFactoryTest.java b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/AggregationFactoryTest.java index 3b37e2077ad..12b93cbac47 100644 --- a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/AggregationFactoryTest.java +++ b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/AggregationFactoryTest.java @@ -11,11 +11,14 @@ import io.opentelemetry.sdk.autoconfigure.internal.SpiHelper; import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.Aggregation; import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.Base2ExponentialBucketHistogram; +import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.Drop; import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.ExplicitBucketHistogram; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.stream.Stream; +import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.LastValue; +import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.Sum; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -45,13 +48,13 @@ private static Stream createTestCases() { Arguments.of( new Aggregation(), io.opentelemetry.sdk.metrics.Aggregation.defaultAggregation()), Arguments.of( - new Aggregation().withDrop(new Object()), + new Aggregation().withDrop(new Drop()), io.opentelemetry.sdk.metrics.Aggregation.drop()), Arguments.of( - new Aggregation().withSum(new Object()), + new Aggregation().withSum(new Sum()), io.opentelemetry.sdk.metrics.Aggregation.sum()), Arguments.of( - new Aggregation().withLastValue(new Object()), + new Aggregation().withLastValue(new LastValue()), io.opentelemetry.sdk.metrics.Aggregation.lastValue()), Arguments.of( new Aggregation() diff --git a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ConfigurationReaderTest.java b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ConfigurationReaderTest.java index 9df4b15d25f..21b65d02e97 100644 --- a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ConfigurationReaderTest.java +++ b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ConfigurationReaderTest.java @@ -44,8 +44,10 @@ import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.TracerProvider; import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.View; import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.Zipkin; +import java.io.ByteArrayInputStream; import java.io.FileInputStream; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -276,4 +278,59 @@ void read_KitchenSinkExampleFile() throws IOException { assertThat(config).isEqualTo(expected); } } + + @Test + void nullValuesParsedToEmptyObjects() { + String objectPlaceholderString = + "file_format: \"0.1\"\n" + + "tracer_provider:\n" + + " processors:\n" + + " - batch:\n" + + " exporter:\n" + + " console: {}\n" + + "meter_provider:\n" + + " views:\n" + + " - selector:\n" + + " instrument_type: histogram\n" + + " stream:\n" + + " aggregation:\n" + + " drop: {}\n"; + OpenTelemetryConfiguration objectPlaceholderModel = + ConfigurationReader.parse( + new ByteArrayInputStream(objectPlaceholderString.getBytes(StandardCharsets.UTF_8))); + + String noOjbectPlaceholderString = + "file_format: \"0.1\"\n" + + "tracer_provider:\n" + + " processors:\n" + + " - batch:\n" + + " exporter:\n" + + " console:\n" + + "meter_provider:\n" + + " views:\n" + + " - selector:\n" + + " instrument_type: histogram\n" + + " stream:\n" + + " aggregation:\n" + + " drop:\n"; + OpenTelemetryConfiguration noObjectPlaceholderModel = + ConfigurationReader.parse( + new ByteArrayInputStream(noOjbectPlaceholderString.getBytes(StandardCharsets.UTF_8))); + + SpanExporter exporter = + noObjectPlaceholderModel + .getTracerProvider() + .getProcessors() + .get(0) + .getBatch() + .getExporter(); + assertThat(exporter.getConsole()).isNotNull(); + assertThat(exporter.getOtlp()).isNull(); + + Aggregation aggregation= noObjectPlaceholderModel.getMeterProvider().getViews().get(0).getStream().getAggregation(); + assertThat(aggregation.getDrop()).isNotNull(); + assertThat(aggregation.getSum()).isNull(); + + assertThat(objectPlaceholderModel).isEqualTo(noObjectPlaceholderModel); + } } From ce52ab45c2b163079b9a60bef8877664fa24e018 Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Wed, 13 Sep 2023 14:43:58 -0500 Subject: [PATCH 2/3] Spotless --- buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts | 5 ++++- .../incubator/fileconfig/ConfigurationReader.java | 1 - .../incubator/fileconfig/AggregationFactoryTest.java | 7 +++---- .../incubator/fileconfig/ConfigurationReaderTest.java | 3 ++- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts b/buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts index b4df8594f02..bf63d7db4c5 100644 --- a/buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts +++ b/buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts @@ -74,7 +74,10 @@ tasks { // https://groups.google.com/forum/#!topic/bazel-discuss/_R3A9TJSoPM "-Xlint:-processing", // We suppress the "options" warning because it prevents compilation on modern JDKs - "-Xlint:-options" + "-Xlint:-options", + + // Fail build on any warning + "-Werror", ), ) } diff --git a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ConfigurationReader.java b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ConfigurationReader.java index fab876a8447..4fe765ee344 100644 --- a/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ConfigurationReader.java +++ b/sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ConfigurationReader.java @@ -8,7 +8,6 @@ import com.fasterxml.jackson.annotation.JsonSetter; import com.fasterxml.jackson.annotation.Nulls; import com.fasterxml.jackson.databind.ObjectMapper; -import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.Aggregation; import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.OpenTelemetryConfiguration; import java.io.InputStream; import org.snakeyaml.engine.v2.api.Load; diff --git a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/AggregationFactoryTest.java b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/AggregationFactoryTest.java index 12b93cbac47..62c5e457606 100644 --- a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/AggregationFactoryTest.java +++ b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/AggregationFactoryTest.java @@ -13,12 +13,12 @@ import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.Base2ExponentialBucketHistogram; import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.Drop; import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.ExplicitBucketHistogram; +import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.LastValue; +import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.Sum; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.stream.Stream; -import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.LastValue; -import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.Sum; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -51,8 +51,7 @@ private static Stream createTestCases() { new Aggregation().withDrop(new Drop()), io.opentelemetry.sdk.metrics.Aggregation.drop()), Arguments.of( - new Aggregation().withSum(new Sum()), - io.opentelemetry.sdk.metrics.Aggregation.sum()), + new Aggregation().withSum(new Sum()), io.opentelemetry.sdk.metrics.Aggregation.sum()), Arguments.of( new Aggregation().withLastValue(new LastValue()), io.opentelemetry.sdk.metrics.Aggregation.lastValue()), diff --git a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ConfigurationReaderTest.java b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ConfigurationReaderTest.java index 21b65d02e97..27d6e14cc0a 100644 --- a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ConfigurationReaderTest.java +++ b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/ConfigurationReaderTest.java @@ -327,7 +327,8 @@ void nullValuesParsedToEmptyObjects() { assertThat(exporter.getConsole()).isNotNull(); assertThat(exporter.getOtlp()).isNull(); - Aggregation aggregation= noObjectPlaceholderModel.getMeterProvider().getViews().get(0).getStream().getAggregation(); + Aggregation aggregation = + noObjectPlaceholderModel.getMeterProvider().getViews().get(0).getStream().getAggregation(); assertThat(aggregation.getDrop()).isNotNull(); assertThat(aggregation.getSum()).isNull(); From 4e1588e3a4595d555397231dde469c4c7b9c185a Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Thu, 14 Sep 2023 08:51:18 -0500 Subject: [PATCH 3/3] Use open-telemetry/opentelemetry-configuration ref --- sdk-extensions/incubator/build.gradle.kts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk-extensions/incubator/build.gradle.kts b/sdk-extensions/incubator/build.gradle.kts index 3dffd250461..7a07f3c8fa6 100644 --- a/sdk-extensions/incubator/build.gradle.kts +++ b/sdk-extensions/incubator/build.gradle.kts @@ -55,8 +55,8 @@ dependencies { // ... proceed with normal sourcesJar, compileJava, etc // TODO(jack-berg): update ref to be released version when available -val configurationRef = "ad219187cdb03c8d8ea1a2311afd280bf64d2262" -val configurationRepoZip = "https://github.com/jack-berg/opentelemetry-configuration/archive/$configurationRef.zip" +val configurationRef = "0508846f82ed54b230fa638e1e7556c52efee25e" +val configurationRepoZip = "https://github.com/open-telemetry/opentelemetry-configuration/archive/$configurationRef.zip" val buildDirectory = layout.buildDirectory.asFile.get() val downloadConfigurationSchema by tasks.registering(Download::class) {