-
Notifications
You must be signed in to change notification settings - Fork 791
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove instrumentation (and tooling) specific properties from Config #1286
Remove instrumentation (and tooling) specific properties from Config #1286
Conversation
0f91c3c
to
502782c
Compare
Regarding #1254 (comment): I've checked all usages and everything besides the Kafka one looks OK. In case of Kafka unit tests change the configuration over and over again during a single scenario which makes it impossible to store the boolean flag in a simple static final constant. I've tried several things but nothing worked out. I guess we're left with three options:
|
javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/config/ConfigBuilder.java
Show resolved
Hide resolved
testing-common/src/main/groovy/io/opentelemetry/auto/test/AgentTestRunner.java
Outdated
Show resolved
Hide resolved
...-api/src/main/java/io/opentelemetry/instrumentation/api/context/ContextPropagationDebug.java
Outdated
Show resolved
Hide resolved
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.
Thanks @mateuszrzeszutek - I think for Kafka, if the tests can be rewritten to only set config once per test, which I guess is a normal pattern, that would be nice, but can be another PR
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/config/Config.java
Show resolved
Hide resolved
...-api/src/main/java/io/opentelemetry/instrumentation/api/context/ContextPropagationDebug.java
Outdated
Show resolved
Hide resolved
...entation-api/src/main/java/io/opentelemetry/instrumentation/api/decorator/BaseDecorator.java
Show resolved
Hide resolved
javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/Instrumenter.java
Show resolved
Hide resolved
I agree -- after thinking about it I believe this is the way to go. Especially if we want to use full agent jar in tests -- in that case dynamically changing property value in the middle of the scenario would be pretty difficult. Added issue: #1292 |
502782c
to
1728207
Compare
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.
馃憤
Resolves #1277
I like how the
Config
class looks like now 馃槃