Idea: deprecate Config, add agent-only InstrumentationConfig#6264
Conversation
| String value = config.getString("otel.instrumentation.experimental.span-suppression-strategy"); | ||
| if (value != null) { | ||
| System.setProperty("otel.instrumentation.experimental.span-suppression-strategy", value); | ||
| } |
There was a problem hiding this comment.
If there are distros setting the default value of this, we need to copy the value over to system properties so that instrumentation-api code can get the correct value.
| /** | ||
| * TraceConfig Instrumentation does not extend Default. | ||
| * | ||
| * <p>Instead it directly implements Instrumenter#instrument() and adds one default Instrumenter for | ||
| * every configured class+method-list. | ||
| * | ||
| * <p>If this becomes a more common use case the building logic should be abstracted out into a | ||
| * super class. | ||
| */ |
There was a problem hiding this comment.
I found a super outdated comment 😄
| @Override | ||
| public void configure(Map<String, ?> configs) {} | ||
| public void configure(Map<String, ?> configs) { | ||
| // TODO: support experimental attributes config |
There was a problem hiding this comment.
Later, in a separate PR, we should refactor this classes so that they use a similar way to configure themselves as introduced in #6138 -- it seems to be the "idiomatic" approach for kafka extensions, I guess
| ConfigPropertiesUtil.getBoolean( | ||
| "otel.instrumentation.aws-sdk.experimental-span-attributes", false)) |
There was a problem hiding this comment.
library-autoconfigure modules have to keep on using env var/system prop configuration, as it's their only way of configuring themselves.
trask
left a comment
There was a problem hiding this comment.
This seems like a good approach until we have an opportunity to do something more radical with autoconfigure's config
|
Okay, I'll mark this PR as ready for review. There's quite a lot of changes to be done to deprecate |
7cdc80a to
745ce4c
Compare
... and bridge
InstrumentationConfigcalls to SDK'sConfigProperties. I tried drafting something that implements ideas from #6250, i.e.: makingConfig/InstrumentationConfigan internal, agent-only class.