Skip to content
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

Change db.statement to only be collected if there is sanitization #3127

Merged
merged 28 commits into from
Apr 6, 2023

Conversation

avzis
Copy link
Contributor

@avzis avzis commented Jan 22, 2023

Add a recommendation to disable DB_STATEMENT by default.

Fixes #3104

Changes

Currently DB_STATEMENT parameter is collecting the full query that is being made to a DB.
I suggest disabling this attribute by default, and giving users the option to opt-in into logging it.

It is also possible to give users a way to supply a sanitization function, in order to display only specific information.

@avzis avzis changed the title Add a recommendation to sanitize DB_STATEMENT in order to prever secu… Add a recommendation to sanitize DB_STATEMENT by default Jan 22, 2023
CHANGELOG.md Outdated Show resolved Hide resolved
semantic_conventions/trace/database.yaml Outdated Show resolved Hide resolved
@avzis avzis requested review from arminru and joaopgrassi and removed request for arminru and joaopgrassi January 26, 2023 11:59
@joaopgrassi joaopgrassi self-requested a review January 27, 2023 16:48
@joaopgrassi
Copy link
Member

@avzis I think this looks good.. I had my approve but then saw it's still in draft. Do you want to do more changes?

@carlosalberto
Copy link
Contributor

This PR can stop being a draft btw (unless you plan to add more content, as Joao mentioned). That way you should way more reviews ;)

@avzis avzis marked this pull request as ready for review January 29, 2023 08:26
@avzis avzis requested review from a team January 29, 2023 08:26
@trask trask changed the title Disable collecting DB_STATEMENT by default Change db.statement to only be collected if there is sanitization Apr 5, 2023
@carlosalberto
Copy link
Contributor

@trask I will merge this and make it part of the April release, unless you think we should hold it for be May one. Let me know what you think.

@trask
Copy link
Member

trask commented Apr 5, 2023

@trask I will merge this and make it part of the April release

👍

@reyang reyang merged commit 10f79bd into open-telemetry:main Apr 6, 2023
@reyang reyang mentioned this pull request Apr 6, 2023
jack-berg pushed a commit that referenced this pull request Apr 6, 2023
Relocate the changelog for #3127.
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:semantic-conventions Related to semantic conventions semconv:database spec:trace Related to the specification/trace directory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

db.statement sanitization default behavior