Skip to content

Conversation

@mznet
Copy link
Contributor

@mznet mznet commented Nov 4, 2025

Description

The existing sanitize_body can only handle limited types of queries, and sometimes it loses the query structure while sanitizing. It also cannot handle tricky queries like aggregation or script.
I improved the body sanitization by masking all values in the leaf nodes. It’s a simple solution, but more clear, no exception, and also keeps the query structure.

Fixes #3876

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

I added new queries like term, script, and aggregation to check if sanitize_body can sanitize well and not lose the query structure. I also checked that the existing test queries still keep their structure while masking.

Does This PR Require a Core Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 4, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: xrmx / name: Riccardo Magliocchetti (1beeab2)

@mznet mznet force-pushed the better-elasticsearch-sanitization branch from fc4629a to 62e324e Compare November 4, 2025 13:36
@xrmx xrmx moved this to Ready for review in @xrmx's Python PR digest Nov 4, 2025
@xrmx
Copy link
Contributor

xrmx commented Nov 4, 2025

@mznet thanks, please add a changelog entry

@mznet mznet requested a review from a team as a code owner November 5, 2025 00:30
@mznet mznet force-pushed the better-elasticsearch-sanitization branch from d5610ea to 1674df0 Compare November 5, 2025 00:33
@mznet
Copy link
Contributor Author

mznet commented Nov 5, 2025

@xrmx i've updated it.

@mznet
Copy link
Contributor Author

mznet commented Nov 5, 2025

@xrmx some tests were failed but i fixed them

@xrmx
Copy link
Contributor

xrmx commented Nov 5, 2025

@mznet please run tox -e pyXX-test-instrumentation-elasticsearch-1 and tox -e lint-instrumentation-elasticsearch locally before the next push

@xrmx xrmx enabled auto-merge (squash) November 10, 2025 09:17
@xrmx xrmx merged commit bd3c1f2 into open-telemetry:main Nov 10, 2025
647 checks passed
@github-project-automation github-project-automation bot moved this from Ready for review to Done in @xrmx's Python PR digest Nov 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Insufficient sanitization in Elasticsearch query handling

2 participants