Apache Geode instrumentation should store normalized queries as db.st…

…atement (#1455) * Apache Geode instrumentation should store normalized queries as db.statement * spotless
open-telemetry · Oct 26, 2020 · 01f4086 · 01f4086
1 parent 3db872e
commit 01f4086
Show file tree

Hide file tree

Showing 4 changed files with 97 additions and 21 deletions.
diff --git a/.../src/main/java/io/opentelemetry/javaagent/instrumentation/geode/GeodeInstrumentation.java b/.../src/main/java/io/opentelemetry/javaagent/instrumentation/geode/GeodeInstrumentation.java
@@ -40,7 +40,7 @@ public ElementMatcher<? super TypeDescription> typeMatcher() {
   @Override
   public String[] helperClassNames() {
     return new String[] {
-      packageName + ".GeodeTracer",
+      packageName + ".GeodeQueryNormalizer", packageName + ".GeodeTracer",
     };
   }
 

diff --git a/.../src/main/java/io/opentelemetry/javaagent/instrumentation/geode/GeodeQueryNormalizer.java b/.../src/main/java/io/opentelemetry/javaagent/instrumentation/geode/GeodeQueryNormalizer.java
@@ -0,0 +1,29 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.geode;
+
+import io.opentelemetry.javaagent.instrumentation.api.db.normalizer.ParseException;
+import io.opentelemetry.javaagent.instrumentation.api.db.normalizer.SqlNormalizer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public final class GeodeQueryNormalizer {
+  private static final Logger log = LoggerFactory.getLogger(GeodeQueryNormalizer.class);
+
+  public static String normalize(String query) {
+    if (query == null) {
+      return null;
+    }
+    try {
+      return SqlNormalizer.normalize(query);
+    } catch (ParseException e) {
+      log.debug("Could not normalize Geode query", e);
+      return null;
+    }
+  }
+
+  private GeodeQueryNormalizer() {}
+}
diff --git a/...geode-1.4/src/main/java/io/opentelemetry/javaagent/instrumentation/geode/GeodeTracer.java b/...geode-1.4/src/main/java/io/opentelemetry/javaagent/instrumentation/geode/GeodeTracer.java
@@ -37,7 +37,7 @@ public Span startSpan(String operation, Region<?, ?> connection, String query) {
 
   @Override
   protected String normalizeQuery(String query) {
-    return query;
+    return GeodeQueryNormalizer.normalize(query);
   }
 
   @Override

diff --git a/instrumentation/geode-1.4/src/test/groovy/PutGetTest.groovy b/instrumentation/geode-1.4/src/test/groovy/PutGetTest.groovy
@@ -9,6 +9,7 @@ import static io.opentelemetry.trace.Span.Kind.INTERNAL
 
 import io.opentelemetry.instrumentation.test.AgentTestRunner
 import io.opentelemetry.trace.attributes.SemanticAttributes
+import org.apache.geode.DataSerializable
 import org.apache.geode.cache.client.ClientCacheFactory
 import org.apache.geode.cache.client.ClientRegionShortcut
 import spock.lang.Shared
@@ -27,11 +28,10 @@ class PutGetTest extends AgentTestRunner {
 
   def "test put and get"() {
     when:
-    def cacheValue
-    runUnderTrace("someTrace") {
+    def cacheValue = runUnderTrace("someTrace") {
       region.clear()
       region.put(key, value)
-      cacheValue = region.get(key)
+      region.get(key)
     }
 
     then:
@@ -68,11 +68,10 @@ class PutGetTest extends AgentTestRunner {
 
   def "test query"() {
     when:
-    def cacheValue
-    runUnderTrace("someTrace") {
+    def cacheValue = runUnderTrace("someTrace") {
       region.clear()
       region.put(key, value)
-      cacheValue = region.query("SELECT * FROM /test-region")
+      region.query("SELECT * FROM /test-region")
     }
 
     then:
@@ -89,11 +88,10 @@ class PutGetTest extends AgentTestRunner {
 
   def "test existsValue"() {
     when:
-    def exists
-    runUnderTrace("someTrace") {
+    def exists = runUnderTrace("someTrace") {
       region.clear()
       region.put(key, value)
-      exists = region.existsValue("SELECT * FROM /test-region")
+      region.existsValue("SELECT * FROM /test-region")
     }
 
     then:
@@ -121,36 +119,85 @@ class PutGetTest extends AgentTestRunner {
           kind CLIENT
           errored false
           attributes {
-            "${SemanticAttributes.DB_SYSTEM.key()}" "geode"
-            "${SemanticAttributes.DB_NAME.key()}" "test-region"
-            "${SemanticAttributes.DB_OPERATION.key()}" "clear"
+            "$SemanticAttributes.DB_SYSTEM.key" "geode"
+            "$SemanticAttributes.DB_NAME.key" "test-region"
+            "$SemanticAttributes.DB_OPERATION.key" "clear"
           }
         }
         span(2) {
           name "put"
           kind CLIENT
           errored false
           attributes {
-            "${SemanticAttributes.DB_SYSTEM.key()}" "geode"
-            "${SemanticAttributes.DB_NAME.key()}" "test-region"
-            "${SemanticAttributes.DB_OPERATION.key()}" "put"
+            "$SemanticAttributes.DB_SYSTEM.key" "geode"
+            "$SemanticAttributes.DB_NAME.key" "test-region"
+            "$SemanticAttributes.DB_OPERATION.key" "put"
           }
         }
         span(3) {
           name verb
           kind CLIENT
           errored false
           attributes {
-            "${SemanticAttributes.DB_SYSTEM.key()}" "geode"
-            "${SemanticAttributes.DB_NAME.key()}" "test-region"
-            "${SemanticAttributes.DB_OPERATION.key()}" verb
+            "$SemanticAttributes.DB_SYSTEM.key" "geode"
+            "$SemanticAttributes.DB_NAME.key" "test-region"
+            "$SemanticAttributes.DB_OPERATION.key" verb
             if (query != null) {
-              "${SemanticAttributes.DB_STATEMENT.key()}" query
+              "$SemanticAttributes.DB_STATEMENT.key" query
             }
           }
         }
       }
     }
     return true
   }
+
+  def "should sanitize geode query"() {
+    given:
+    def value = new Card(cardNumber: '1234432156788765', expDate: '10/2020')
+
+    region.clear()
+    region.put(1, value)
+    TEST_WRITER.waitForTraces(2)
+    TEST_WRITER.clear()
+
+    when:
+    def results = region.query("SELECT * FROM /test-region p WHERE p.expDate = '10/2020'")
+
+    then:
+    results.toList() == [value]
+
+    assertTraces(1) {
+      trace(0, 1) {
+        span(0) {
+          name "query"
+          kind CLIENT
+          errored false
+          attributes {
+            "$SemanticAttributes.DB_SYSTEM.key" "geode"
+            "$SemanticAttributes.DB_NAME.key" "test-region"
+            "$SemanticAttributes.DB_OPERATION.key" "query"
+            "$SemanticAttributes.DB_STATEMENT.key" "SELECT * FROM /test-region p WHERE p.expDate = ?"
+          }
+        }
+      }
+    }
+  }
+
+  static class Card implements DataSerializable {
+    String cardNumber
+    String expDate
+
+    @Override
+    void toData(DataOutput dataOutput) throws IOException {
+      dataOutput.writeUTF(cardNumber)
+      dataOutput.writeUTF(expDate)
+    }
+
+    @Override
+    void fromData(DataInput dataInput) throws IOException, ClassNotFoundException {
+      cardNumber = dataInput.readUTF()
+      expDate = dataInput.readUTF()
+    }
+  }
 }