Opensearch transport query sanitization#15634
Merged
trask merged 25 commits intoopen-telemetry:mainfrom Mar 13, 2026
Merged
Conversation
jaydeluca
reviewed
Dec 15, 2025
jaydeluca
reviewed
Dec 15, 2025
| | System property | Type | Default | Description | | ||
| |-----------------------------------------------------------------|---------| ------- |------------------------------------------------------| | ||
| | `otel.instrumentation.opensearch.experimental-span-attributes` | Boolean | `false` | Enable the capture of experimental span attributes. | | ||
| | `otel.instrumentation.opensearch.capture-search-query` | Boolean | `false` | Enable the capture of sanitized search query bodies. | |
Member
There was a problem hiding this comment.
I wonder if it's worth mentioning that this will likely impact performance in some way due to the serialization?
Suggested change
| | `otel.instrumentation.opensearch.capture-search-query` | Boolean | `false` | Enable the capture of sanitized search query bodies. | | |
| | `otel.instrumentation.opensearch.capture-search-query` | Boolean | `false` | Enable the capture of sanitized search query bodies. **Note**: Enabling this feature adds overhead for JSON serialization and parsing on search requests. | |
Contributor
Author
There was a problem hiding this comment.
@jaydeluca I think so. This feature can add overhead to search requests. The overhead is expected to be slight, but could be unexpected on systems that use a lot of search requests. It's better to clarify this.
- remove duplicated code - fix wrong comment
laurit
reviewed
Dec 18, 2025
laurit
reviewed
Dec 18, 2025
laurit
reviewed
Dec 18, 2025
Contributor
Author
|
The build failed due to a link validation check on armeria.dev, which is returning 404 not found(line/armeria#6560) |
|
armeria.dev is back online. We apologize for the inconvenience. 🙇 |
breedx-splk
reviewed
Jan 6, 2026
laurit
reviewed
Jan 7, 2026
laurit
reviewed
Feb 26, 2026
Comment on lines
+37
to
+44
| if (request instanceof GenericSerializable) { | ||
| // GenericSerializable writes directly to output stream, cannot sanitize | ||
| // This path is typically not used for search queries | ||
| ByteArrayOutputStream baos = new ByteArrayOutputStream(); | ||
| ((GenericSerializable) request).serialize(baos); | ||
| String body = baos.toString(UTF_8); | ||
| return body.isEmpty() ? null : body; | ||
| } |
Contributor
There was a problem hiding this comment.
none of the tests seem to use this part
Contributor
Author
There was a problem hiding this comment.
@laurit since the type of request is checked to be either SearchRequest or MsearchRequest before calling extractSanitized, so the request can't be an instance of GenericSerializable. it was not a dead code before, but it became the dead code after some modification. it should be removed.
laurit
reviewed
Feb 26, 2026
| | System property | Type | Default | Description | | ||
| | -------------------------------------------------------------- | ------- | ------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | `otel.instrumentation.opensearch.experimental-span-attributes` | Boolean | `false` | Enable the capture of experimental span attributes. | | ||
| | `otel.instrumentation.opensearch.capture-search-query` | Boolean | `false` | Enable the capture of sanitized search query bodies. **Note**: Enabling this feature adds overhead for JSON serialization and parsing on search requests. | |
Contributor
There was a problem hiding this comment.
Since capturing the query is now simpler than in the initial version I'm wondering whether we should add the performance note at all. Perhaps we should enable this by default?
cc @trask
Member
There was a problem hiding this comment.
seems reasonable, it's definitely useful data
laurit
approved these changes
Mar 4, 2026
jaydeluca
approved these changes
Mar 9, 2026
Contributor
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
6 participants
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.
Since extracting query body can impact application performance, I made it optional with
otel.instrumentation.elasticsearch.capture-search-query. Not sure if adding this config is okay though.close #15466