Add ability to customize span exception handling to instrumenter#18530
Open
jaydeluca wants to merge 7 commits into
Open
Add ability to customize span exception handling to instrumenter#18530jaydeluca wants to merge 7 commits into
jaydeluca wants to merge 7 commits into
Conversation
This was referenced May 3, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new SpanExceptionHandler extension point on Instrumenter to customize how exception events are recorded, and uses it in JDBC and Lettuce to sanitize exception messages/stacktraces when query sanitization is enabled (addressing sensitive-data leakage concerns raised in #8422 / #3039).
Changes:
- Add
SpanExceptionHandlerAPI and plumb it throughInstrumenterBuilder→Instrumenterend logic. - Apply sanitizing exception handlers in JDBC and Lettuce when query sanitization is enabled.
- Add focused tests to verify sanitized vs. unsanitized behavior.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/SpanExceptionHandler.java | New public SPI for customizing exception event recording. |
| instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/DefaultSpanExceptionHandler.java | Default implementation delegating to Span.recordException. |
| instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/InstrumenterBuilder.java | Adds builder setter/state for SpanExceptionHandler. |
| instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/Instrumenter.java | Uses SpanExceptionHandler when ending spans with errors. |
| instrumentation/lettuce/lettuce-5.1/library/src/main/java/.../LettuceTelemetryBuilder.java | Configures sanitizing exception handler when query sanitization is enabled. |
| instrumentation/lettuce/lettuce-5.1/library/src/main/java/.../LettuceSanitizingSpanExceptionHandler.java | New handler that redacts Lettuce RedisCommandExecutionException details. |
| instrumentation/lettuce/lettuce-5.1/library/src/test/java/.../LettuceExceptionSanitizationTest.java | New tests covering sanitized vs unsanitized Lettuce exception events. |
| instrumentation/jdbc/library/src/main/java/.../JdbcInstrumenterFactory.java | Wires sanitizing exception handler into statement + transaction instrumenters. |
| instrumentation/jdbc/library/src/main/java/.../JdbcSanitizingSpanExceptionHandler.java | New handler that sanitizes SQLException exception event fields. |
| instrumentation/jdbc/library/src/main/java/.../datasource/JdbcTelemetryBuilder.java | Passes query-sanitization flag to transaction instrumenter construction. |
| instrumentation/jdbc/library/src/test/java/.../JdbcExceptionSanitizationTest.java | New tests covering JDBC exception sanitization behavior and type preservation. |
| instrumentation/jdbc/library/src/test/java/.../internal/OpenTelemetryConnectionTest.java | Updates transaction instrumenter factory call signature. |
| instrumentation/jdbc/javaagent/src/main/java/.../JdbcSingletons.java | Ensures transaction instrumenter gets query-sanitization flag in javaagent wiring. |
| docs/apidiffs/current_vs_latest/opentelemetry-instrumentation-api.txt | Captures the new public API surface in the API diff report. |
Comment on lines
+27
to
+34
| // Tracer.Span.start() takes no args in lettuce 5.x, but requires RedisCommand in 6.x+. | ||
| private static void startSpan(Tracer.Span span) { | ||
| try { | ||
| Method startMethod = span.getClass().getMethod("start"); | ||
| startMethod.invoke(span); | ||
| } catch (ReflectiveOperationException e) { | ||
| throw new RuntimeException(e); | ||
| } |
Comment on lines
+28
to
+41
| String sanitizedMsg = "<redacted>"; | ||
|
|
||
| StringBuilder stackTrace = new StringBuilder(); | ||
| stackTrace.append(throwable.getClass().getName()).append(": ").append(sanitizedMsg); | ||
| for (StackTraceElement element : throwable.getStackTrace()) { | ||
| stackTrace.append("\n\tat ").append(element); | ||
| } | ||
|
|
||
| span.addEvent( | ||
| "exception", | ||
| Attributes.of( | ||
| EXCEPTION_TYPE, throwable.getClass().getName(), | ||
| EXCEPTION_MESSAGE, sanitizedMsg, | ||
| EXCEPTION_STACKTRACE, stackTrace.toString())); |
Comment on lines
+33
to
+48
| String state = sql.getSQLState(); | ||
| String sanitizedMsg = | ||
| "SQL error [" + sql.getErrorCode() + (state != null ? "/" + state : "") + "]"; | ||
|
|
||
| StringBuilder stackTrace = new StringBuilder(); | ||
| stackTrace.append(throwable.getClass().getName()).append(": ").append(sanitizedMsg); | ||
| for (StackTraceElement element : throwable.getStackTrace()) { | ||
| stackTrace.append("\n\tat ").append(element); | ||
| } | ||
|
|
||
| span.addEvent( | ||
| "exception", | ||
| Attributes.of( | ||
| EXCEPTION_TYPE, throwable.getClass().getName(), | ||
| EXCEPTION_MESSAGE, sanitizedMsg, | ||
| EXCEPTION_STACKTRACE, stackTrace.toString())); |
Comment on lines
106
to
118
| public static Instrumenter<DbRequest, Void> createTransactionInstrumenter( | ||
| OpenTelemetry openTelemetry) { | ||
| @SuppressWarnings("deprecation") // using deprecated config property | ||
| boolean enabled = | ||
| DeclarativeConfigUtil.getInstrumentationConfig(openTelemetry, "jdbc") | ||
| .get("transaction/development") | ||
| .getBoolean( | ||
| "enabled", | ||
| ConfigPropertiesUtil.getBoolean( | ||
| "otel.instrumentation.jdbc.experimental.transaction.enabled", false)); | ||
| return createTransactionInstrumenter(openTelemetry, enabled); | ||
| boolean querySanitizationEnabled = DbConfig.isQuerySanitizationEnabled(openTelemetry, "jdbc"); | ||
| return createTransactionInstrumenter(openTelemetry, enabled, querySanitizationEnabled); | ||
| } |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #8422 and is related to open-telemetry/opentelemetry-specification#3496 and #3039
I had originally experimented with using a
ErrorCauseExtractor, but it had a limitation of not preserving the original exception type.In terms of the implementations of the JDBC sanitization, it is similar to how node handles it as discussed in #8422 (comment), but it's not handled consistently across all their differnt database types
Implemented it on JDBC and lettuce, but it might make sense for others too if we think this is a reasonable approach