-
Notifications
You must be signed in to change notification settings - Fork 1k
make declarative config bridge usable by spring starter and contrib #14497
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
Changes from all commits
4ee8844
973616a
2d45769
a3cdc34
cf4cec7
ddf64e5
b261076
59f1c4d
c5bf4e6
2caa7f0
7a69707
1f8e84a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
# OpenTelemetry Instrumentation API Incubator | ||
|
||
Instrumentation API Incubator is a collection of libraries that provide additional functionality | ||
for OpenTelemetry instrumentation and auto-configuration. It is intended to be used by | ||
instrumentation authors and auto-configuration providers to enhance their capabilities and provide a | ||
more consistent experience when working with OpenTelemetry. | ||
|
||
## Declarative Config Bridge | ||
|
||
Declarative Config Bridge allows instrumentation authors to access configuration in a uniform way, | ||
regardless of the configuration source. | ||
|
||
The bridge allows you to read configuration using the system property style when dealing with | ||
declarative configuration. | ||
|
||
### Example | ||
|
||
As an example, let's look at the inferred spans configuration. | ||
First, there is a configuration method that reads the properties and is unaware of the source of the | ||
configuration: | ||
|
||
```java | ||
class InferredSpansConfig { | ||
static SpanProcessor create(ConfigProperties properties) { | ||
// read properties here | ||
boolean backupDiagnosticFiles = | ||
properties.getBoolean("otel.inferred.spans.backup.diagnostic.files", false); | ||
} | ||
} | ||
``` | ||
|
||
The auto configuration **without declarative config** passes the provided properties directly: | ||
|
||
```java | ||
|
||
@AutoService(AutoConfigurationCustomizerProvider.class) | ||
public class InferredSpansAutoConfig implements AutoConfigurationCustomizerProvider { | ||
|
||
@Override | ||
public void customize(AutoConfigurationCustomizer config) { | ||
config.addTracerProviderCustomizer( | ||
(providerBuilder, properties) -> { | ||
providerBuilder.addSpanProcessor(InferredSpansConfig.create(properties)); | ||
return providerBuilder; | ||
}); | ||
} | ||
} | ||
``` | ||
|
||
The auto configuration **with declarative config** uses the Declarative Config Bridge to be able to | ||
use common configuration method: | ||
|
||
Let's first look at the yaml file that is used to configure the inferred spans processor: | ||
|
||
```yaml | ||
file_format: 1.0-rc.1 | ||
tracer_provider: | ||
processors: | ||
- inferred_spans: | ||
backup: | ||
diagnostic: | ||
files: true | ||
``` | ||
And now the component provider that uses the Declarative Config Bridge: | ||
```java | ||
|
||
@AutoService(ComponentProvider.class) | ||
public class InferredSpansComponentProvider implements ComponentProvider<SpanProcessor> { | ||
|
||
@Override | ||
public String getName() { | ||
return "inferred_spans"; | ||
} | ||
|
||
@Override | ||
public SpanProcessor create(DeclarativeConfigProperties config) { | ||
return InferredSpansConfig.create( | ||
new DeclarativeConfigPropertiesBridgeBuilder() | ||
// crop the prefix, because the properties are under the "inferred_spans" processor | ||
.addMapping("otel.inferred.spans.", "") | ||
.build(config)); | ||
} | ||
|
||
@Override | ||
public Class<SpanProcessor> getType() { | ||
return SpanProcessor.class; | ||
} | ||
} | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.instrumentation.api.incubator.sdk.config.bridge; | ||
|
||
public final class ConfigPropertiesUtil { | ||
private ConfigPropertiesUtil() {} | ||
|
||
public static String propertyYamlPath(String propertyName) { | ||
return yamlPath(propertyName); | ||
} | ||
|
||
static String yamlPath(String property) { | ||
String[] segments = DeclarativeConfigPropertiesBridge.getSegments(property); | ||
if (segments.length == 0) { | ||
throw new IllegalArgumentException("Invalid property: " + property); | ||
} | ||
|
||
return "'instrumentation/development' / 'java' / '" + String.join("' / '", segments) + "'"; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.instrumentation.api.incubator.sdk.config.bridge; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
|
||
import org.junit.jupiter.api.Test; | ||
|
||
class ConfigPropertiesUtilTest { | ||
@Test | ||
void propertyYamlPath() { | ||
assertThat(ConfigPropertiesUtil.propertyYamlPath("google.otel.auth.target.signals")) | ||
.isEqualTo( | ||
"'instrumentation/development' / 'java' / 'google' / 'otel' / 'auth' / 'target' / 'signals'"); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
file_format: 0.4 | ||
file_format: 1.0-rc.1 | ||
instrumentation/development: | ||
java: | ||
acme: | ||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -57,7 +57,6 @@ dependencies { | |||
bootstrapLibs(project(":instrumentation-api")) | ||||
// opentelemetry-api is an api dependency of :instrumentation-api, but opentelemetry-api-incubator is not | ||||
bootstrapLibs("io.opentelemetry:opentelemetry-api-incubator") | ||||
bootstrapLibs(project(":instrumentation-api-incubator")) | ||||
bootstrapLibs(project(":instrumentation-annotations-support")) | ||||
bootstrapLibs(project(":javaagent-bootstrap")) | ||||
|
||||
|
@@ -71,8 +70,11 @@ dependencies { | |||
exclude("io.opentelemetry", "opentelemetry-sdk-extension-autoconfigure-spi") | ||||
} | ||||
baseJavaagentLibs(project(":javaagent-extension-api")) | ||||
baseJavaagentLibs(project(":instrumentation-api-incubator")) | ||||
|
||||
baseJavaagentLibs(project(":javaagent-tooling")) | ||||
baseJavaagentLibs(project(":javaagent-tooling")) { | ||||
exclude("io.opentelemetry", "opentelemetry-sdk-extension-autoconfigure-spi") | ||||
} | ||||
baseJavaagentLibs(project(":javaagent-internal-logging-application")) | ||||
baseJavaagentLibs(project(":javaagent-internal-logging-simple", configuration = "shadow")) | ||||
baseJavaagentLibs(project(":muzzle")) | ||||
|
@@ -147,8 +149,7 @@ tasks { | |||
val buildBootstrapLibs by registering(ShadowJar::class) { | ||||
configurations = listOf(bootstrapLibs) | ||||
|
||||
// exclude the agent part of the javaagent-extension-api; these classes will be added in relocate tasks | ||||
exclude("io/opentelemetry/javaagent/extension/**") | ||||
excludeNonBootstrapClasses() | ||||
|
||||
duplicatesStrategy = DuplicatesStrategy.EXCLUDE | ||||
|
||||
|
@@ -286,7 +287,8 @@ tasks { | |||
doLast { | ||||
val filePath = rootDir.toPath().resolve("licenses").resolve("licenses.md") | ||||
if (Files.exists(filePath)) { | ||||
val datePattern = Pattern.compile("^_[0-9]{4}-[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2} .*_$") | ||||
val datePattern = | ||||
Pattern.compile("^_[0-9]{4}-[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}:[0-9]{2} .*_$") | ||||
val lines = Files.readAllLines(filePath) | ||||
// 4th line contains the timestamp of when the license report was generated, replace it with | ||||
// an empty line | ||||
|
@@ -412,7 +414,8 @@ fun CopySpec.copyByteBuddy(jar: Provider<RegularFile>) { | |||
eachFile { | ||||
if (path.startsWith("net/bytebuddy/") && | ||||
// this is our class that we have placed in the byte buddy package, need to preserve it | ||||
!path.startsWith("net/bytebuddy/agent/builder/AgentBuilderUtil")) { | ||||
!path.startsWith("net/bytebuddy/agent/builder/AgentBuilderUtil") | ||||
) { | ||||
exclude() | ||||
} else if (path.startsWith("META-INF/versions/9/net/bytebuddy/")) { | ||||
path = path.removePrefix("META-INF/versions/9/") | ||||
|
@@ -422,17 +425,30 @@ fun CopySpec.copyByteBuddy(jar: Provider<RegularFile>) { | |||
} | ||||
} | ||||
|
||||
// exclude bootstrap projects from javaagent libs - they won't be added to inst/ | ||||
fun ShadowJar.excludeNonBootstrapClasses() { | ||||
// exclude the agent part of the javaagent-extension-api; these classes will be added in relocate tasks | ||||
exclude("io/opentelemetry/javaagent/extension/**") | ||||
exclude("**/instrumentation/api/incubator/sdk/**") | ||||
} | ||||
|
||||
// exclude bootstrap projects from javaagent libs - they won't be added to inst/ | ||||
fun ShadowJar.excludeBootstrapClasses() { | ||||
dependencies { | ||||
exclude(project(":instrumentation-api")) | ||||
exclude(project(":instrumentation-api-incubator")) | ||||
exclude(project(":instrumentation-annotations-support")) | ||||
exclude(project(":javaagent-bootstrap")) | ||||
} | ||||
|
||||
// exclude the bootstrap part of the javaagent-extension-api | ||||
exclude("io/opentelemetry/javaagent/bootstrap/**") | ||||
|
||||
// all in instrumentation-api-incubator except the bridge package | ||||
exclude("io/opentelemetry/instrumentation/api/incubator/builder/**") | ||||
exclude("io/opentelemetry/instrumentation/api/incubator/config/**") | ||||
exclude("io/opentelemetry/instrumentation/api/incubator/instrumenter/**") | ||||
exclude("io/opentelemetry/instrumentation/api/incubator/log/**") | ||||
exclude("io/opentelemetry/instrumentation/api/incubator/semconv/**") | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if a new package was added and we forgot to add an exclusion for it here, would something fail in a way we'd know to add it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this would fail Line 61 in 7a69707
It failed for me until I changed this at least. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cool, i was wondering how that change was related |
||||
} | ||||
|
||||
class JavaagentProvider( | ||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be worth adding for extra clarity that this allows to use declarative/yaml configuration in addition to the java properties and environment variables, in short we have to change how configuration is consumed to support both, not how it's being set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, if you use declarative config you can't use env vars any more
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do the java properties still work ? Also, do we still have a way to handle compatibility with existing dotted syntax even when configuration options do not strictly fit yaml structure ? (disclamer: I have little knowledge about declarative configuration, maybe this is documented somewhere).
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not by default - but you can add that capability back.
The migration schema provides the same capabilities as before, e.g.
value: ${OTEL_SERVICE_NAME:-unknown_service}
adds support for the env var and sys property for the service nameno worries - the user facing documentation is not there yet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only thing that I worry (at least for now) is having the capability to preserve the current support for environment variables and JVM system properties in a distribution (also includes some elements from contrib), so any documentation how to achieve this would be welcome. If users decide to use declarative configuration, then it's a breaking change and we don't have such requirement (and they should be able to use system properties and env variables in yaml if needed).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we're not changing anything for user that are not using declarative config