From a1d26223e2c2e14659d045e2a26937e3e7e320e2 Mon Sep 17 00:00:00 2001
From: xi xia <xxia@newrelic.com>
Date: Wed, 1 Dec 2021 14:16:36 -0800
Subject: [PATCH] remove todo and add comments

---
 .../agent/sql/DefaultSlowQueryListener.java       |  4 ++--
 .../newrelic/agent/tracers/DefaultSqlTracer.java  | 15 ++++++++-------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java b/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java
index 7e56688e68..6f2820ce2a 100644
--- a/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java
+++ b/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java
@@ -55,8 +55,8 @@ public <T> void noticeTracer(Tracer tracer, SlowQueryDatastoreParameters<T> slow
             }
 
             // This allows transaction traces to show slow queries directly in the trace details
-            //todo: obfuscatedQueryString isn't correct, it could be Raw or Obfuscated. The queryConverter
-            // has conditional obfuscation, respecting recrod_sql setting.
+            // Unfortunately, SQL_OBFUSCATED_PARAMETER_NAME is misleading. Before reaching here, the query has already
+            // been processed by SqlQueryConverter in DefaultSqlTracer.  The result could be Raw, Obfuscated, or null.
             tracer.setAgentAttribute(SqlTracer.SQL_OBFUSCATED_PARAMETER_NAME, query);
 
             DatastoreConfig datastoreConfig = ServiceFactory.getConfigService().getDefaultAgentConfig().getDatastoreConfig();
diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java b/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java
index 8e1aa8bfec..e9a60a7b73 100644
--- a/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java
+++ b/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java
@@ -305,13 +305,8 @@ protected void recordMetrics(TransactionStats transactionStats) {
     }
 
     /**
-     * This method is named this way because the docs on toObfuscatedQueryString imply obfuscation certainty.
-     * The reality is the SqlQueryConverter implementation of this method has introduced conditional
-     * logic that respects the agent configuration setting `record_sql`.
-     *
-     * If OBFUSCATED, sql is obfuscated.
-     * If RAW, sql is returned, unchanged.
-     * If OFF, null is returned.
+     * This method is named this way because {@link SqlQueryConverter#toObfuscatedQueryString(String)} has
+     * unexpected conditional logic.
      */
     private String getNoRawOrObfuscatedSql(String rawSql, String appName) {
         SqlQueryConverter converter = new SqlQueryConverter(appName, getDatabaseVendor());
@@ -559,6 +554,12 @@ public String toRawQueryString(String rawQuery) {
             return rawQuery;
         }
 
+        /**
+         * For this implementation, the getSqlfuscator has conditional
+         * logic to return an obfuscator that respects the agent configuration setting `record_sql`
+         *
+         * See {@link com.newrelic.agent.database.DatabaseService#createSqlObfuscator(TransactionTracerConfig) }
+         */
         @Override
         public String toObfuscatedQueryString(String rawQuery) {
             SqlObfuscator sqlObfuscator = ServiceFactory.getDatabaseService().getSqlObfuscator(appName);