-
Notifications
You must be signed in to change notification settings - Fork 324
Improve config mapping for OpenTelemetry extensions #7193
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
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
c3f87a8
Support arbitrary lookup of 'otel.' config from extensions
mcculls c506daa
Use property for OTel API version
mcculls 5a1c2dd
Don't embed unnecessary classes
mcculls 8b7b749
Use includes to control which areas of the OTel API we embed
mcculls 62e9b8e
Rework OTel package remapper to remap both shaded and unshaded refere…
mcculls e79db83
Embed OTel instrumentation API, removing classes already covered by e…
mcculls 6f16d9f
Embed OTel javaagent-extension API, removing classes already covered …
mcculls 3312aea
Embed opentelemetry-javaagent-servlet-common-bootstrap
mcculls 528089e
Redirect InstrumentationConfig requests to our own ConfigProvider
mcculls 3a6894e
Ignore partial build class-path when checking for forbidden APIs in o…
mcculls File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
33 changes: 0 additions & 33 deletions
33
...t-otel/otel-bootstrap/src/main/java/datadog/trace/bootstrap/otel/Java8BytecodeBridge.java
This file was deleted.
Oops, something went wrong.
24 changes: 24 additions & 0 deletions
24
.../otel-bootstrap/src/main/java/datadog/trace/bootstrap/otel/instrumentation/CallDepth.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| package datadog.trace.bootstrap.otel.instrumentation; | ||
|
|
||
| import datadog.trace.bootstrap.CallDepthThreadLocalMap; | ||
|
|
||
| /** Redirects requests to our own {@link CallDepthThreadLocalMap}. */ | ||
| public final class CallDepth { | ||
| private final Class<?> clazz; | ||
|
|
||
| private CallDepth(final Class<?> clazz) { | ||
| this.clazz = clazz; | ||
| } | ||
|
|
||
| public static CallDepth forClass(Class<?> clazz) { | ||
| return new CallDepth(clazz); | ||
| } | ||
|
|
||
| public int getAndIncrement() { | ||
| return CallDepthThreadLocalMap.incrementCallDepth(clazz); | ||
| } | ||
|
|
||
| public int decrementAndGet() { | ||
| return CallDepthThreadLocalMap.decrementCallDepth(clazz); | ||
| } | ||
| } |
93 changes: 93 additions & 0 deletions
93
...ain/java/datadog/trace/bootstrap/otel/instrumentation/internal/InstrumentationConfig.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| package datadog.trace.bootstrap.otel.instrumentation.internal; | ||
|
|
||
| import datadog.trace.bootstrap.config.provider.ConfigProvider; | ||
| import java.time.Duration; | ||
| import java.util.Collections; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Set; | ||
| import java.util.regex.Matcher; | ||
| import java.util.regex.Pattern; | ||
|
|
||
| /** Redirects requests to our own {@link ConfigProvider}. */ | ||
| public final class InstrumentationConfig { | ||
| private static final InstrumentationConfig INSTANCE = new InstrumentationConfig(); | ||
|
|
||
| private static final Pattern DURATION_PATTERN = Pattern.compile("(\\d+)(ms|[DdHhMmSs]?)"); | ||
|
|
||
| private static final ConfigProvider delegate = ConfigProvider.getInstance(); | ||
|
|
||
| public static InstrumentationConfig get() { | ||
| return INSTANCE; | ||
| } | ||
|
|
||
| public String getString(String name) { | ||
| return delegate.getString(name); | ||
| } | ||
|
|
||
| public String getString(String name, String defaultValue) { | ||
| return delegate.getString(name, defaultValue); | ||
| } | ||
|
|
||
| public boolean getBoolean(String name, boolean defaultValue) { | ||
| return delegate.getBoolean(name, defaultValue); | ||
| } | ||
|
|
||
| public int getInt(String name, int defaultValue) { | ||
| return delegate.getInteger(name, defaultValue); | ||
| } | ||
|
|
||
| public long getLong(String name, long defaultValue) { | ||
| return delegate.getLong(name, defaultValue); | ||
| } | ||
|
|
||
| public double getDouble(String name, double defaultValue) { | ||
| return delegate.getDouble(name, defaultValue); | ||
| } | ||
|
|
||
| public Duration getDuration(String name, Duration defaultValue) { | ||
| String durationString = delegate.getString(name); | ||
| if (null == durationString) { | ||
| return defaultValue; | ||
| } | ||
| Matcher matcher = DURATION_PATTERN.matcher(durationString); | ||
| if (matcher.matches()) { | ||
| long value = Integer.parseInt(matcher.group(1)); | ||
| String unit = matcher.group(2); | ||
| if ("D".equalsIgnoreCase(unit)) { | ||
| return Duration.ofDays(value); | ||
| } else if ("H".equalsIgnoreCase(unit)) { | ||
| return Duration.ofHours(value); | ||
| } else if ("M".equalsIgnoreCase(unit)) { | ||
| return Duration.ofMinutes(value); | ||
| } else if ("S".equalsIgnoreCase(unit)) { | ||
| return Duration.ofSeconds(value); | ||
| } else { | ||
| return Duration.ofMillis(value); // already in ms | ||
| } | ||
| } else { | ||
| throw new IllegalArgumentException( | ||
| "Invalid duration property " + name + "=" + durationString); | ||
| } | ||
| } | ||
|
|
||
| public List<String> getList(String name) { | ||
| return getList(name, Collections.emptyList()); | ||
| } | ||
|
|
||
| public List<String> getList(String name, List<String> defaultValue) { | ||
| return delegate.getList(name, defaultValue); | ||
| } | ||
|
|
||
| public Set<String> getSet(String name, Set<String> defaultValue) { | ||
| return delegate.getSet(name, defaultValue); | ||
| } | ||
|
|
||
| public Map<String, String> getMap(String name, Map<String, String> defaultValue) { | ||
| Map<String, String> map = delegate.getMergedMap(name); | ||
| if (map.isEmpty()) { | ||
| map = defaultValue; | ||
| } | ||
| return map; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Just checking: can this cause an issue with customers shipping OTel SDK (especially another version) as part of their dependencies?
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.
no, the embedded classpath is shaded - see https://github.com/DataDog/dd-trace-java/blob/mcculls/map-otel-config-for-dropin/dd-java-agent/agent-otel/otel-bootstrap/build.gradle#L97
the only code which will use it are external extensions which are loaded and translated by our extension mechanism
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.
Between custom Gradle configuration, shadowing and
ClassRemapperimplementation, I wonder if there is a better way (in term of maintainability) to express the single need of "I want this dependency under this package" 🤔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.
You could look at using a single configuration file - the downside is that you'd need more code on both the gradle and Java sides to load that file into the respective plugins, increasing their complexity. Sometimes duplication is acceptable for things that change infrequently...