Skip to content

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Jdbc declarative config params #15199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1b8db66
add declarative config parameters
zeitlinger Oct 31, 2025
709e4f2
add declarative config parameters
zeitlinger Oct 31, 2025
cebb7a5
lint
zeitlinger Nov 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/OpenTelemetryDriver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ public Connection connect(String url, Properties info) throws SQLException {
Instrumenter<DbRequest, Void> statementInstrumenter =
JdbcInstrumenterFactory.createStatementInstrumenter(openTelemetry);

boolean captureQueryParameters = JdbcInstrumenterFactory.captureQueryParameters();
boolean captureQueryParameters = JdbcInstrumenterFactory.captureQueryParameters(openTelemetry);
Instrumenter<DbRequest, Void> transactionInstrumenter =
JdbcInstrumenterFactory.createTransactionInstrumenter(openTelemetry);

Expand Down
80 changes: 70 additions & 10 deletions ...src/main/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcInstrumenterFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@

package io.opentelemetry.instrumentation.jdbc.internal;

import static io.opentelemetry.api.incubator.config.DeclarativeConfigProperties.empty;
import static java.util.Collections.emptyList;

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.api.incubator.ExtendedOpenTelemetry;
import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties;
import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeAttributesExtractor;
import io.opentelemetry.instrumentation.api.incubator.semconv.code.CodeSpanNameExtractor;
import io.opentelemetry.instrumentation.api.incubator.semconv.db.DbClientMetrics;
Expand All @@ -32,14 +35,30 @@ public final class JdbcInstrumenterFactory {
private static final JdbcNetworkAttributesGetter netAttributesGetter =
new JdbcNetworkAttributesGetter();

public static boolean captureQueryParameters() {
return ConfigPropertiesUtil.getBoolean(
"otel.instrumentation.jdbc.experimental.capture-query-parameters", false);
public static boolean captureQueryParameters(OpenTelemetry openTelemetry) {
if (openTelemetry instanceof ExtendedOpenTelemetry) {
ExtendedOpenTelemetry extendedOpenTelemetry = (ExtendedOpenTelemetry) openTelemetry;

DeclarativeConfigProperties instrumentationConfig =
extendedOpenTelemetry.getConfigProvider().getInstrumentationConfig();

if (instrumentationConfig == null) {
return false;
}

return instrumentationConfig
Copy link
Member

@jeanbisutti jeanbisutti Nov 3, 2025

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?

Copy link
Member Author

@zeitlinger zeitlinger Nov 3, 2025

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

.getStructured("jdbc", empty())
.getStructured("experimental", empty())
.getBoolean("capture-query-parameters", false);
} else {
return ConfigPropertiesUtil.getBoolean(
"otel.instrumentation.jdbc.experimental.capture-query-parameters", false);
}
Copy link
Member

@trask trask Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the declarative config syntax, I think it's ok that it's a bit verbose, but it's our forward looking config API and so I think it's ok to lean into it

I don't mind the if/else, but if we are going to bridge, I'd prefer to bridge in the other direction where we back the new declarative config API with a fake model based on the system properties, so that instrumentation only needs to use the official API

(this would actually be very nice I think if it works, to push instrumentation into the new config API)

}

public static Instrumenter<DbRequest, Void> createStatementInstrumenter(
OpenTelemetry openTelemetry) {
return createStatementInstrumenter(openTelemetry, captureQueryParameters());
return createStatementInstrumenter(openTelemetry, captureQueryParameters(openTelemetry));
}

static Instrumenter<DbRequest, Void> createStatementInstrumenter(
Expand All @@ -48,8 +67,7 @@ static Instrumenter<DbRequest, Void> createStatementInstrumenter(
openTelemetry,
emptyList(),
true,
ConfigPropertiesUtil.getBoolean(
"otel.instrumentation.common.db-statement-sanitizer.enabled", true),
statementSanitizationEnabled(openTelemetry, true),
captureQueryParameters);
}

Expand Down Expand Up @@ -97,10 +115,8 @@ public static Instrumenter<DataSource, DbInfo> createDataSourceInstrumenter(

public static Instrumenter<DbRequest, Void> createTransactionInstrumenter(
OpenTelemetry openTelemetry) {
return createTransactionInstrumenter(
openTelemetry,
ConfigPropertiesUtil.getBoolean(
"otel.instrumentation.jdbc.experimental.transaction.enabled", false));

return createTransactionInstrumenter(openTelemetry, transactionEnabled(openTelemetry, false));
}

public static Instrumenter<DbRequest, Void> createTransactionInstrumenter(
Expand All @@ -122,5 +138,49 @@ public static Instrumenter<DbRequest, Void> createTransactionInstrumenter(
.buildInstrumenter(SpanKindExtractor.alwaysClient());
}

private static boolean transactionEnabled(OpenTelemetry openTelemetry, boolean defaultEnabled) {
if (openTelemetry instanceof ExtendedOpenTelemetry) {
Copy link
Member

@jeanbisutti jeanbisutti Nov 3, 2025

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);
}

ExtendedOpenTelemetry extendedOpenTelemetry = (ExtendedOpenTelemetry) openTelemetry;

DeclarativeConfigProperties instrumentationConfig =
extendedOpenTelemetry.getConfigProvider().getInstrumentationConfig();

if (instrumentationConfig == null) {
return defaultEnabled;
}

return instrumentationConfig
.getStructured("jdbc", empty())
.getStructured("experimental", empty())
.getStructured("transaction", empty())
.getBoolean("enabled", defaultEnabled);
} else {
return ConfigPropertiesUtil.getBoolean(
"otel.instrumentation.jdbc.experimental.transaction.enabled", defaultEnabled);
}
}

private static boolean statementSanitizationEnabled(
OpenTelemetry openTelemetry, boolean defaultEnabled) {
if (openTelemetry instanceof ExtendedOpenTelemetry) {
ExtendedOpenTelemetry extendedOpenTelemetry = (ExtendedOpenTelemetry) openTelemetry;

DeclarativeConfigProperties instrumentationConfig =
extendedOpenTelemetry.getConfigProvider().getInstrumentationConfig();

if (instrumentationConfig == null) {
return defaultEnabled;
}

return instrumentationConfig
.getStructured("common", empty())
.getStructured("db-statement-sanitizer", empty())
.getBoolean("enabled", defaultEnabled);
} else {
return ConfigPropertiesUtil.getBoolean(
"otel.instrumentation.common.db-statement-sanitizer.enabled", defaultEnabled);
}
}

private JdbcInstrumenterFactory() {}
}
Loading