fix!: Return message when sql is over the obfuscation limit (#1149)

Previously, when a SQL query with a prepended comment
exceeded the obfuscation limit, the query would be truncated without
obfuscation.

Now, when the when the obfuscation limit is hit, a message is 
returned and obfuscation is not attempted.

Author: kaylareopelle

helpers/sql-obfuscation/README.md

@@ -25,7 +25,7 @@ end
 
 Make sure the `Instrumentation` class for your gem contains configuration options for:
 
-- `:obfuscation_limit`: the length at which the obfuscated SQL string will be truncated.
+- `:obfuscation_limit`: the length at which the SQL string will not be obfuscated
   Example: `option :obfuscation_limit, default: 2000, validate: :integer`
 
 If you want to add support for a new adapter, update the following constants to include keys for your adapter:

helpers/sql-obfuscation/lib/opentelemetry/helpers/sql_obfuscation.rb

@@ -94,14 +94,16 @@ def generate_regex(dialect)
       # This is a SQL obfuscation utility intended for use in database adapter instrumentation.
       #
       # @param sql [String] The SQL to obfuscate.
-      # @param obfuscation_limit [optional Integer] The maximum length of an obfuscated sql statement.
+      # @param obfuscation_limit [optional Integer] the length at which the SQL string will not be obfuscated
       # @param adapter [optional Symbol] the type of database adapter calling the method. `:default`, `:mysql` and `:postgres` are supported.
       # @return [String] The SQL query string where the values are replaced with "?". When the sql statement exceeds the obufscation limit
       #  the first matched pair from the SQL statement will be returned, with an appended truncation message. If trunaction is unsuccessful,
       #  a string describing the error will be returned.
       #
       # @api public
       def obfuscate_sql(sql, obfuscation_limit: 2000, adapter: :default)
+        return "SQL not obfuscated, query exceeds #{obfuscation_limit} characters" if sql.size > obfuscation_limit
+
         regex = case adapter
                 when :mysql
                   MYSQL_COMPONENTS_REGEX
@@ -115,7 +117,6 @@ def obfuscate_sql(sql, obfuscation_limit: 2000, adapter: :default)
         # https://github.com/open-telemetry/opentelemetry-ruby-contrib/pull/160
         # https://github.com/open-telemetry/opentelemetry-ruby-contrib/pull/345
         sql = OpenTelemetry::Common::Utilities.utf8_encode(sql, binary: true)
-        return truncate_statement(sql, regex, obfuscation_limit) if sql.size > obfuscation_limit
 
         sql = sql.gsub(regex, PLACEHOLDER)
         return 'Failed to obfuscate SQL query - quote characters remained after obfuscation' if CLEANUP_REGEX[adapter].match(sql)
@@ -124,16 +125,6 @@ def obfuscate_sql(sql, obfuscation_limit: 2000, adapter: :default)
       rescue StandardError => e
         OpenTelemetry.handle_error(message: 'Failed to obfuscate SQL', exception: e)
       end
-
-      # @api private
-      def truncate_statement(sql, regex, limit)
-        first_match_index = sql.index(regex)
-        truncation_message = "SQL truncated (> #{limit} characters)"
-        return truncation_message unless first_match_index
-
-        truncated_sql = sql[..first_match_index - 1]
-        "#{truncated_sql}...\n#{truncation_message}"
-      end
     end
   end
 end

helpers/sql-obfuscation/test/helpers/sql_obfuscation_test.rb

@@ -18,17 +18,9 @@ def test_named_arg_defaults_obfuscates
     assert_equal(expected, result)
   end
 
-  def test_obfuscation_limit_truncates_query_after_first_match
+  def test_obfuscation_returns_message_when_limit_is_reached
     sql = "SELECT * from users where users.id = 1 and users.email = '[email protected]'"
-    expected = "SELECT * from users where users.id = ...\nSQL truncated (> 42 characters)"
-    result = OpenTelemetry::Helpers::SqlObfuscation.obfuscate_sql(sql, obfuscation_limit: 42)
-
-    assert_equal(expected, result)
-  end
-
-  def test_obfuscation_limit_truncates_when_query_not_encoded_with_utf8
-    sql = "SELECT * from 😄 where users.id = 1 and users.😄 = '[email protected]'"
-    expected = "SELECT * from  where users.id = ...\nSQL truncated (> 42 characters)"
+    expected = 'SQL not obfuscated, query exceeds 42 characters'
     result = OpenTelemetry::Helpers::SqlObfuscation.obfuscate_sql(sql, obfuscation_limit: 42)
 
     assert_equal(expected, result)

instrumentation/mysql2/test/opentelemetry/instrumentation/mysql2/instrumentation_test.rb

@@ -236,20 +236,9 @@
       describe 'with obfuscation_limit' do
         let(:config) { { db_statement: :obfuscate, obfuscation_limit: 10 } }
 
-        it 'truncates SQL using config limit' do
+        it 'returns a message when the limit is reached' do
           sql = "SELECT * from users where users.id = 1 and users.email = '[email protected]'"
-          obfuscated_sql = "SELECT * from users where users.id = ...\nSQL truncated (> 10 characters)"
-          expect do
-            client.query(sql)
-          end.must_raise Mysql2::Error
-
-          _(span.attributes['db.statement']).must_equal obfuscated_sql
-        end
-
-        it 'handles regex non-matches' do
-          sql = 'ALTER TABLE my_table DISABLE TRIGGER ALL;'
-          obfuscated_sql = 'SQL truncated (> 10 characters)'
-
+          obfuscated_sql = 'SQL not obfuscated, query exceeds 10 characters'
           expect do
             client.query(sql)
           end.must_raise Mysql2::Error

instrumentation/pg/test/opentelemetry/instrumentation/pg/instrumentation_test.rb

@@ -297,20 +297,9 @@
       describe 'with obfuscation_limit' do
         let(:config) { { db_statement: :obfuscate, obfuscation_limit: 10 } }
 
-        it 'truncates SQL using config limit' do
+        it 'returns a message when the limit is reached' do
           sql = "SELECT * from users where users.id = 1 and users.email = '[email protected]'"
-          obfuscated_sql = "SELECT * from users where users.id = ...\nSQL truncated (> 10 characters)"
-          expect do
-            client.exec(sql)
-          end.must_raise PG::UndefinedTable
-
-          _(span.attributes['db.statement']).must_equal obfuscated_sql
-        end
-
-        it 'handles regex non-matches' do
-          sql = 'ALTER TABLE my_table DISABLE TRIGGER ALL;'
-          obfuscated_sql = 'SQL truncated (> 10 characters)'
-
+          obfuscated_sql = 'SQL not obfuscated, query exceeds 10 characters'
           expect do
             client.exec(sql)
           end.must_raise PG::UndefinedTable

instrumentation/trilogy/test/opentelemetry/instrumentation/trilogy/instrumentation_test.rb

@@ -323,20 +323,9 @@
       describe 'with obfuscation_limit' do
         let(:config) { { db_statement: :obfuscate, obfuscation_limit: 10 } }
 
-        it 'truncates SQL using config limit' do
+        it 'returns a message when the limit is reached' do
           sql = "SELECT * from users where users.id = 1 and users.email = '[email protected]'"
-          obfuscated_sql = "SELECT * from users where users.id = ...\nSQL truncated (> 10 characters)"
-          expect do
-            client.query(sql)
-          end.must_raise Trilogy::Error
-
-          _(span.attributes['db.statement']).must_equal obfuscated_sql
-        end
-
-        it 'handles regex non-matches' do
-          sql = 'ALTER TABLE my_table DISABLE TRIGGER ALL;'
-          obfuscated_sql = 'SQL truncated (> 10 characters)'
-
+          obfuscated_sql = 'SQL not obfuscated, query exceeds 10 characters'
           expect do
             client.query(sql)
           end.must_raise Trilogy::Error