-
Notifications
You must be signed in to change notification settings - Fork 1k
Jdbc declarative config params #15199
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
base: main
Are you sure you want to change the base?
Jdbc declarative config params #15199
Conversation
|
@trask please have a look |
| return false; | ||
| } | ||
|
|
||
| return instrumentationConfig |
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.
Are there tests for declarative configuration applied to the JDBC instrumentation?
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.
to early for that - I first want to discuss if this approach is better than #14767
| } | ||
|
|
||
| private static boolean transactionEnabled(OpenTelemetry openTelemetry, boolean defaultEnabled) { | ||
| if (openTelemetry instanceof ExtendedOpenTelemetry) { |
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.
(minor) The else could be removed in this way:
if (!openTelemetry instanceof ExtendedOpenTelemetry) {
return ConfigPropertiesUtil.getBoolean(
"otel.instrumentation.jdbc.experimental.transaction.enabled", defaultEnabled);
}
I tried out using the declarative config API directly as discussed in #14767
It feels quite verbose - but let's use it as a start for the discussion