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.

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

Initialize file configuration #5399

Merged
merged 12 commits into from
Aug 3, 2023
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import io.opentelemetry.gradle.OtelJavaExtension
import org.gradle.api.plugins.quality.internal.CheckstyleAction
jack-berg marked this conversation as resolved.
Show resolved Hide resolved
import org.gradle.api.tasks.testing.logging.TestExceptionFormat

plugins {
Expand Down
58 changes: 57 additions & 1 deletion sdk-extensions/incubator/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
import de.undercouch.gradle.tasks.download.Download

plugins {
id("otel.java-conventions")
id("otel.publish-conventions")

id("otel.jmh-conventions")
id("otel.animalsniffer-conventions")

id("de.undercouch.download")
id("org.jsonschema2pojo")
}

// SDK modules that are still being developed.
Expand All @@ -22,12 +27,63 @@ dependencies {
implementation(project(":sdk-extensions:autoconfigure-spi"))
implementation("org.snakeyaml:snakeyaml-engine")

// io.opentelemetry.sdk.extension.trace.zpages
// io.opentelemetry.sdk.extension.incubator.trace.zpages
implementation(project(":semconv"))
compileOnly("com.sun.net.httpserver:http")

// io.opentelemetry.sdk.extension.incubator.fileconfig
implementation("com.fasterxml.jackson.core:jackson-databind")
implementation("com.fasterxml.jackson.dataformat:jackson-dataformat-yaml")
Copy link
Member

Choose a reason for hiding this comment

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

Will this eventually make jackson a hard dependency for the sdk or just this extension?

Copy link
Member

Choose a reason for hiding this comment

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

jackson-databind gets CVEs very often, I'd sleep better if we don't include it in the SDK (at some point); would jackson-jr be able to handle the generated POJOs?

Copy link
Member Author

Choose a reason for hiding this comment

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

This would likely eventually be merged into the autoconfigure module, rather than the SDK itself.

I can investigate jackson-jr.

Also, there's the more hands on option of doing something like we do with YAML view configuration, where we use snake YAML to parse to a generic type, then extract the fields manually. Maybe we use jackson-databind / jackson-jr to prototype, and consider switching to the manual method when its looking more complete.

Copy link
Member

Choose a reason for hiding this comment

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

Considering this is for configuration, which is likely only done at startup I feel we should go with a potentially less performant option and not use jackson in exchange for fewer dependencies and CVE exposure.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's less of the performance I'm worried about and more the additional complexity to manually do the parsing that jackson does automatically. Additionally, there is likely to be a fair amount of churn in the early versions of the configuration schema - I'm happy enough writing manual parsing code once, but won't sign up to rewrite the manual parsing code for each of the early iterations.


testImplementation(project(":sdk:testing"))
testImplementation(project(":sdk-extensions:autoconfigure"))

testImplementation("com.google.guava:guava-testlib")
}

val configurationRef = "74607f261e1f9a2ef356b7f87e394b6e9dd90285"
val configurationRepoZip = "https://github.com/open-telemetry/opentelemetry-configuration/archive/$configurationRef.zip"

val downloadConfigurationSchema by tasks.registering(Download::class) {
src(configurationRepoZip)
dest("$buildDir/configuration/opentelemetry-configuration.zip")
overwrite(false)
}

val downloadAndUnzipConfigurationSchema by tasks.registering(Copy::class) {
dependsOn("downloadConfigurationSchema")
jack-berg marked this conversation as resolved.
Show resolved Hide resolved
from(zipTree(downloadConfigurationSchema.get().dest))
eachFile(closureOf<FileCopyDetails> {
// Remove the top level folder "/opentelemetry-configuration-$configurationRef"
val pathParts = path.split("/")
path = pathParts.subList(1, pathParts.size).joinToString("/")
})
into("$buildDir/configuration/")
}

jsonSchema2Pojo {
sourceFiles = setOf(file("$buildDir/configuration/schema"))
targetDirectory = file("$buildDir/generated/sources/js2p/java/main")
targetPackage = "io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model"
includeSetters = false
removeOldOutput = true
}

tasks.getByName("generateJsonSchema2Pojo").dependsOn(downloadAndUnzipConfigurationSchema)
tasks.getByName("sourcesJar").dependsOn("generateJsonSchema2Pojo")

// Exclude jsonschema2pojo generated sources from checkstyle
tasks.named<Checkstyle>("checkstyleMain") {
exclude("**/fileconfig/internal/model/**")
}

tasks {
withType<Test>().configureEach {
environment(
mapOf(
// Expose the kitchen sink example file to tests
"CONFIG_EXAMPLE_FILE" to "$buildDir/configuration/kitchen-sink-example.yaml"
)
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.sdk.extension.incubator.fileconfig;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.OpentelemetryConfiguration;
import java.io.IOException;
import java.io.InputStream;

class ConfigurationReader {

private static final ObjectMapper MAPPER = new ObjectMapper(new YAMLFactory());

private ConfigurationReader() {}

/**
* Parse the {@code configuration} YAML and return the {@link OpentelemetryConfiguration}.
*
* @throws IOException if unable to parse
*/
static OpentelemetryConfiguration parse(InputStream configuration) throws IOException {
return MAPPER.readValue(configuration, OpentelemetryConfiguration.class);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.sdk.extension.incubator.fileconfig;

import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.OpentelemetryConfiguration;
import java.io.FileInputStream;
import java.io.IOException;
import org.junit.jupiter.api.Test;

class ConfigurationReaderTest {

@Test
void read_ExampleFile() throws IOException {
try (FileInputStream configExampleFile =
new FileInputStream(System.getenv("CONFIG_EXAMPLE_FILE"))) {
OpentelemetryConfiguration configuration = ConfigurationReader.parse(configExampleFile);

assertThat(configuration.getFileFormat()).isEqualTo("0.1");
}
}
}
2 changes: 2 additions & 0 deletions settings.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ pluginManagement {
id("com.github.ben-manes.versions") version "0.46.0"
id("com.github.johnrengelman.shadow") version "8.1.1"
id("com.gradle.enterprise") version "3.13"
id("de.undercouch.download") version "5.4.0"
id("org.jsonschema2pojo") version "1.2.1"
id("io.github.gradle-nexus.publish-plugin") version "1.3.0"
id("org.graalvm.buildtools.native") version "0.9.21"
}
Expand Down