fix!: Return message when sql is over the obfuscation limit #1149
+12
−62
Conversation
This file contains 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
Previously, when a SQL query with a prepended comment exceeded the obfuscation limit, the query would be truncated without obfuscation. Now, when the obfuscator detects a prepended comment in a query that needs to be truncated, the prepended comment will be replaced with the placeholder and the remaining query will be truncated to the obfuscation limit.
helpers/sql-obfuscation/lib/opentelemetry/helpers/sql_obfuscation.rb
Outdated
Show resolved
Hide resolved
kaylareopelle
commented
Sep 4, 2024
helpers/sql-obfuscation/lib/opentelemetry/helpers/sql_obfuscation.rb
Outdated
Show resolved
Hide resolved
When the obfusaction limit is hit, return a message stating this and do not attempt to obfuscate
This obfuscation limit matches what New Relic uses for their Ruby agent. The previous 2000 limit seemed arbitrary.
kaylareopelle
changed the title
kaylareopelle
changed the title
Don't even set the regex adapter if the SQL's over the limit, just return the message and get out of there
kaylareopelle
changed the title
|
Ready? |
kaylareopelle
requested review from
fbogsany,
mwear,
robertlaurin,
dazuma,
ericmustin,
ahayworth,
plantfansam,
robbkidd,
simi and
xuan-cao-swi
as code owners
|
Whoops, yes, @arielvalentin! Opened now. |
|
cc: @reid-rigo Making you aware that we are reverting this functionality. We will be looking to restore it in the near future, but we have identified an issue with sanitizing SQL queries that prepend comments, so we have to revert it now. |
arielvalentin
approved these changes
Sep 9, 2024
mwear
approved these changes
Sep 10, 2024
xuan-cao-swi
approved these changes
Sep 10, 2024
Closed
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.
When SQL queries had prepended comments and were over the configured obfuscation limit, the query would fail to obfuscate and sensitive data could be exposed.
The previous approach was incompatible because the first index with a match would be the zero index, so the range would return the entire SQL string and append the truncation message to the end of it.
Until we can find a way to make the first match approach work with prepended queries, let's return a message stating the query was over the limit.
Resolves #1146