clean up PR, rename variables and refactor test logic for readability

jordan-wong · jordan-wong · commit 69b16cc7afb4 · 2025-01-17T15:49:11.000-05:00
diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/SQLCommenter.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/SQLCommenter.java
@@ -19,7 +19,7 @@ public class SQLCommenter {
   private static final String DATABASE_SERVICE = encode("dddbs");
   private static final String DD_HOSTNAME = encode("ddh");
   private static final String DD_DB_NAME = encode("dddb");
-  private static final String DD_DB_INSTANCE = "ddprs";
+  private static final String DD_PEER_SERVICE = "ddprs";
   private static final String DD_ENV = encode("dde");
   private static final String DD_VERSION = encode("ddpv");
   private static final String TRACEPARENT = encode("traceparent");
@@ -100,9 +100,9 @@ public static String inject(
     }
 
     AgentSpan currSpan = activeSpan();
-    Object peerService = null;
+    Object peerServiceObj = null;
     if (currSpan != null) {
-      peerService = currSpan.getTag(Tags.PEER_SERVICE);
+      peerServiceObj = currSpan.getTag(Tags.PEER_SERVICE);
     }
 
     final Config config = Config.get();
@@ -112,7 +112,7 @@ public static String inject(
     final int commentSize = capacity(traceParent, parentService, dbService, env, version);
     StringBuilder sb = new StringBuilder(sql.length() + commentSize);
     boolean commentAdded = false;
-    String peerServiceString = peerService != null ? peerService.toString() : null;
+    String peerService = peerServiceObj != null ? peerServiceObj.toString() : null;
 
     if (appendComment) {
       sb.append(sql);
@@ -126,7 +126,7 @@ public static String inject(
               dbService,
               hostname,
               dbName,
-              peerServiceString,
+              peerService,
               env,
               version,
               traceParent);
@@ -141,7 +141,7 @@ public static String inject(
               dbService,
               hostname,
               dbName,
-              peerServiceString,
+              peerService,
               env,
               version,
               traceParent);
@@ -208,38 +208,14 @@ private static String encode(final String val) {
     return val;
   }
 
-  /*
   protected static boolean toComment(
       StringBuilder sb,
       final boolean injectTrace,
       final String parentService,
       final String dbService,
       final String hostname,
       final String dbName,
-      final String env,
-      final String version,
-      final String traceparent) {
-    int emptySize = sb.length();
-    append(sb, PARENT_SERVICE, parentService, false);
-    append(sb, DATABASE_SERVICE, dbService, sb.length() > emptySize);
-    append(sb, DD_HOSTNAME, hostname, sb.length() > emptySize);
-    append(sb, DD_DB_NAME, dbName, sb.length() > emptySize);
-    append(sb, DD_ENV, env, sb.length() > emptySize);
-    append(sb, DD_VERSION, version, sb.length() > emptySize);
-    if (injectTrace) {
-      append(sb, TRACEPARENT, traceparent, sb.length() > emptySize);
-    }
-    return sb.length() > emptySize;
-  }*/
-
-  protected static boolean toComment(
-      StringBuilder sb,
-      final boolean injectTrace,
-      final String parentService,
-      final String dbService,
-      final String hostname,
-      final String dbName,
-      final String dbInstance,
+      final String peerService,
       final String env,
       final String version,
       final String traceparent) {
@@ -249,8 +225,8 @@ protected static boolean toComment(
     append(sb, DATABASE_SERVICE, dbService, sb.length() > emptySize);
     append(sb, DD_HOSTNAME, hostname, sb.length() > emptySize);
     append(sb, DD_DB_NAME, dbName, sb.length() > emptySize);
-    if (dbInstance != null) {
-      append(sb, DD_DB_INSTANCE, dbInstance, sb.length() > emptySize);
+    if (peerService != null) {
+      append(sb, DD_PEER_SERVICE, peerService, sb.length() > emptySize);
     }
     append(sb, DD_ENV, env, sb.length() > emptySize);
     append(sb, DD_VERSION, version, sb.length() > emptySize);
diff --git a/dd-java-agent/instrumentation/jdbc/src/test/groovy/SQLCommenterTest.groovy b/dd-java-agent/instrumentation/jdbc/src/test/groovy/SQLCommenterTest.groovy
@@ -41,15 +41,13 @@ class SQLCommenterTest extends AgentTestRunner {
     String sqlWithComment = ""
     if (injectTrace) {
       sqlWithComment = SQLCommenter.inject(query, dbService, dbType, host, dbName, traceParent, true, appendComment)
-    } else {
-      if (appendComment) {
-        sqlWithComment = SQLCommenter.append(query, dbService, dbType, host, dbName)
-      } else {
-        sqlWithComment = SQLCommenter.prepend(query, dbService, dbType, host, dbName)
-      }
+    } else if (appendComment) {
+      sqlWithComment = SQLCommenter.append(query, dbService, dbType, host, dbName)
+    }
+    else {
+      sqlWithComment = SQLCommenter.prepend(query, dbService, dbType, host, dbName)
     }
 
-    sqlWithComment == expected
 
     then:
     sqlWithComment == expected
@@ -126,15 +124,14 @@ class SQLCommenterTest extends AgentTestRunner {
 
       if (injectTrace) {
         sqlWithComment = SQLCommenter.inject(query, dbService, dbType, host, dbName, traceParent, true, appendComment)
-      } else {
-        if (appendComment) {
-          sqlWithComment = SQLCommenter.append(query, dbService, dbType, host, dbName)
-        } else {
-          sqlWithComment = SQLCommenter.prepend(query, dbService, dbType, host, dbName)
-        }
+      }
+      else if (appendComment) {
+        sqlWithComment = SQLCommenter.append(query, dbService, dbType, host, dbName)
+      }
+      else {
+        sqlWithComment = SQLCommenter.prepend(query, dbService, dbType, host, dbName)
       }
     }
-    sqlWithComment == expected
 
     then:
     sqlWithComment == expected
