Sql sanitizer should by default treat double-quoted fragments as string literals

Author: laurit

instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/config/internal/CommonConfig.java

@@ -34,6 +34,7 @@ public final class CommonConfig {
   private final Set<String> knownHttpRequestMethods;
   private final EnduserConfig enduserConfig;
   private final boolean statementSanitizationEnabled;
+  private final boolean statementSanitizationAnsiQuotes;
   private final boolean sqlCommenterEnabled;
   private final boolean emitExperimentalHttpClientTelemetry;
   private final boolean emitExperimentalHttpServerTelemetry;
@@ -88,6 +89,8 @@ public CommonConfig(InstrumentationConfig config) {
                 new ArrayList<>(HttpConstants.KNOWN_METHODS)));
     statementSanitizationEnabled =
         config.getBoolean("otel.instrumentation.common.db-statement-sanitizer.enabled", true);
+    statementSanitizationAnsiQuotes =
+        config.getBoolean("otel.instrumentation.common.db-statement-sanitizer.ansi-quotes", false);
     sqlCommenterEnabled =
         config.getBoolean(
             "otel.instrumentation.common.experimental.db-sqlcommenter.enabled", false);
@@ -142,6 +145,10 @@ public boolean isStatementSanitizationEnabled() {
     return statementSanitizationEnabled;
   }
 
+  public boolean isStatementSanitizationAnsiQuotes() {
+    return statementSanitizationAnsiQuotes;
+  }
+
   public boolean isSqlCommenterEnabled() {
     return sqlCommenterEnabled;
   }

instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/DbClientSpanNameExtractor.java

@@ -36,7 +36,21 @@ public static <REQUEST> SpanNameExtractor<REQUEST> create(
    */
   public static <REQUEST> SpanNameExtractor<REQUEST> create(
       SqlClientAttributesGetter<REQUEST, ?> getter) {
-    return new SqlClientSpanNameExtractor<>(getter);
+    return create(getter, SqlDialect.DEFAULT);
+  }
+
+  /**
+   * Returns a {@link SpanNameExtractor} that constructs the span name according to DB semantic
+   * conventions: {@code <db.operation> <db.name>.<identifier>}.
+   *
+   * @see SqlStatementInfo#getOperation() used to extract {@code <db.operation>}.
+   * @see DbClientAttributesGetter#getDbNamespace(Object) used to extract {@code <db.namespace>}.
+   * @see SqlStatementInfo#getMainIdentifier() used to extract {@code <db.table>} or stored
+   *     procedure name.
+   */
+  public static <REQUEST> SpanNameExtractor<REQUEST> create(
+      SqlClientAttributesGetter<REQUEST, ?> getter, SqlDialect sqlDialect) {
+    return new SqlClientSpanNameExtractor<>(getter, sqlDialect.ansiQuotes());
   }
 
   private static final String DEFAULT_SPAN_NAME = "DB Query";
@@ -87,14 +101,19 @@ private static final class SqlClientSpanNameExtractor<REQUEST>
       extends DbClientSpanNameExtractor<REQUEST> {
 
     private final SqlClientAttributesGetter<REQUEST, ?> getter;
+    private final boolean ansiQuotes;
 
-    private SqlClientSpanNameExtractor(SqlClientAttributesGetter<REQUEST, ?> getter) {
+    private SqlClientSpanNameExtractor(
+        SqlClientAttributesGetter<REQUEST, ?> getter, boolean ansiQuotes) {
       this.getter = getter;
+      this.ansiQuotes = ansiQuotes;
     }
 
     @Override
     public String extract(REQUEST request) {
       String namespace = getter.getDbNamespace(request);
+      SqlDialect dialect =
+          SqlStatementSanitizerUtil.getDialect(getter.getDbSystem(request), ansiQuotes);
       Collection<String> rawQueryTexts = getter.getRawQueryTexts(request);
 
       if (rawQueryTexts.isEmpty()) {
@@ -106,22 +125,22 @@ public String extract(REQUEST request) {
           return computeSpanName(namespace, null, null);
         }
         SqlStatementInfo sanitizedStatement =
-            SqlStatementSanitizerUtil.sanitize(rawQueryTexts.iterator().next());
+            SqlStatementSanitizerUtil.sanitize(rawQueryTexts.iterator().next(), dialect);
         return computeSpanName(
             namespace, sanitizedStatement.getOperation(), sanitizedStatement.getMainIdentifier());
       }
 
       if (rawQueryTexts.size() == 1) {
         SqlStatementInfo sanitizedStatement =
-            SqlStatementSanitizerUtil.sanitize(rawQueryTexts.iterator().next());
+            SqlStatementSanitizerUtil.sanitize(rawQueryTexts.iterator().next(), dialect);
         String operation = sanitizedStatement.getOperation();
         if (isBatch(request)) {
           operation = "BATCH " + operation;
         }
         return computeSpanName(namespace, operation, sanitizedStatement.getMainIdentifier());
       }
 
-      MultiQuery multiQuery = MultiQuery.analyze(rawQueryTexts, false);
+      MultiQuery multiQuery = MultiQuery.analyze(rawQueryTexts, dialect, false);
       return computeSpanName(
           namespace,
           multiQuery.getOperation() != null ? "BATCH " + multiQuery.getOperation() : "BATCH",

instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/MultiQuery.java

@@ -24,12 +24,13 @@ private MultiQuery(
   }
 
   static MultiQuery analyze(
-      Collection<String> rawQueryTexts, boolean statementSanitizationEnabled) {
+      Collection<String> rawQueryTexts, SqlDialect dialect, boolean statementSanitizationEnabled) {
     UniqueValue uniqueMainIdentifier = new UniqueValue();
     UniqueValue uniqueOperation = new UniqueValue();
     Set<String> uniqueStatements = new LinkedHashSet<>();
     for (String rawQueryText : rawQueryTexts) {
-      SqlStatementInfo sanitizedStatement = SqlStatementSanitizerUtil.sanitize(rawQueryText);
+      SqlStatementInfo sanitizedStatement =
+          SqlStatementSanitizerUtil.sanitize(rawQueryText, dialect);
       String mainIdentifier = sanitizedStatement.getMainIdentifier();
       uniqueMainIdentifier.set(mainIdentifier);
       String operation = sanitizedStatement.getOperation();

instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractor.java

@@ -67,17 +67,20 @@ public static <REQUEST, RESPONSE> SqlClientAttributesExtractorBuilder<REQUEST, R
   private final AttributeKey<String> oldSemconvTableAttribute;
   private final boolean statementSanitizationEnabled;
   private final boolean captureQueryParameters;
+  private final boolean statementSanitizationAnsiQuotes;
 
   SqlClientAttributesExtractor(
       SqlClientAttributesGetter<REQUEST, RESPONSE> getter,
       AttributeKey<String> oldSemconvTableAttribute,
       boolean statementSanitizationEnabled,
+      boolean statementSanitizationAnsiQuotes,
       boolean captureQueryParameters) {
     this.getter = getter;
     this.oldSemconvTableAttribute = oldSemconvTableAttribute;
     // capturing query parameters disables statement sanitization
     this.statementSanitizationEnabled = !captureQueryParameters && statementSanitizationEnabled;
     this.captureQueryParameters = captureQueryParameters;
+    this.statementSanitizationAnsiQuotes = statementSanitizationAnsiQuotes;
     internalNetworkExtractor = new InternalNetworkAttributesExtractor<>(getter, true, false);
     serverAttributesExtractor = ServerAttributesExtractor.create(getter);
   }
@@ -86,14 +89,18 @@ public static <REQUEST, RESPONSE> SqlClientAttributesExtractorBuilder<REQUEST, R
   @Override
   public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST request) {
     Collection<String> rawQueryTexts = getter.getRawQueryTexts(request);
+    SqlDialect dialect =
+        SqlStatementSanitizerUtil.getDialect(
+            getter.getDbSystem(request), statementSanitizationAnsiQuotes);
 
     Long batchSize = getter.getBatchSize(request);
     boolean isBatch = batchSize != null && batchSize > 1;
 
     if (SemconvStability.emitOldDatabaseSemconv()) {
       if (rawQueryTexts.size() == 1) { // for backcompat(?)
         String rawQueryText = rawQueryTexts.iterator().next();
-        SqlStatementInfo sanitizedStatement = SqlStatementSanitizerUtil.sanitize(rawQueryText);
+        SqlStatementInfo sanitizedStatement =
+            SqlStatementSanitizerUtil.sanitize(rawQueryText, dialect);
         String operation = sanitizedStatement.getOperation();
         internalSet(
             attributes,
@@ -112,7 +119,8 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST
       }
       if (rawQueryTexts.size() == 1) {
         String rawQueryText = rawQueryTexts.iterator().next();
-        SqlStatementInfo sanitizedStatement = SqlStatementSanitizerUtil.sanitize(rawQueryText);
+        SqlStatementInfo sanitizedStatement =
+            SqlStatementSanitizerUtil.sanitize(rawQueryText, dialect);
         String operation = sanitizedStatement.getOperation();
         internalSet(
             attributes,
@@ -124,7 +132,8 @@ public void onStart(AttributesBuilder attributes, Context parentContext, REQUEST
         }
       } else if (rawQueryTexts.size() > 1) {
         MultiQuery multiQuery =
-            MultiQuery.analyze(getter.getRawQueryTexts(request), statementSanitizationEnabled);
+            MultiQuery.analyze(
+                getter.getRawQueryTexts(request), dialect, statementSanitizationEnabled);
         internalSet(attributes, DB_QUERY_TEXT, join("; ", multiQuery.getStatements()));
 
         String operation =

instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlClientAttributesExtractorBuilder.java

@@ -21,6 +21,7 @@ public final class SqlClientAttributesExtractorBuilder<REQUEST, RESPONSE> {
   AttributeKey<String> oldSemconvTableAttribute = DB_SQL_TABLE;
   boolean statementSanitizationEnabled = true;
   boolean captureQueryParameters = false;
+  boolean setStatementSanitizationAnsiQuotes = false;
 
   SqlClientAttributesExtractorBuilder(SqlClientAttributesGetter<REQUEST, RESPONSE> getter) {
     this.getter = getter;
@@ -49,6 +50,19 @@ public SqlClientAttributesExtractorBuilder<REQUEST, RESPONSE> setStatementSaniti
     return this;
   }
 
+  /**
+   * Sets whether the SQL sanitizer should treat double-quoted fragments as string literals or
+   * identifiers. By default, double quotes are used for string literals. When the sanitizer is able
+   * to detect that the used database does not support double-quoted string literals then this flag
+   * will be automatically switched.
+   */
+  @CanIgnoreReturnValue
+  public SqlClientAttributesExtractorBuilder<REQUEST, RESPONSE>
+      setSetStatementSanitizationAnsiQuotes(boolean setStatementSanitizationAnsiQuotes) {
+    this.setStatementSanitizationAnsiQuotes = setStatementSanitizationAnsiQuotes;
+    return this;
+  }
+
   /**
    * Sets whether the query parameters should be captured as span attributes named {@code
    * db.query.parameter.<key>}. Enabling this option disables the statement sanitization. Disabled
@@ -70,6 +84,10 @@ public SqlClientAttributesExtractorBuilder<REQUEST, RESPONSE> setCaptureQueryPar
    */
   public AttributesExtractor<REQUEST, RESPONSE> build() {
     return new SqlClientAttributesExtractor<>(
-        getter, oldSemconvTableAttribute, statementSanitizationEnabled, captureQueryParameters);
+        getter,
+        oldSemconvTableAttribute,
+        statementSanitizationEnabled,
+        setStatementSanitizationAnsiQuotes,
+        captureQueryParameters);
   }
 }

instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlDialect.java

@@ -7,7 +7,26 @@
 
 /** Enumeration of sql dialects that are handled differently by {@link SqlStatementSanitizer}. */
 public enum SqlDialect {
-  DEFAULT,
-  // couchbase uses double quotes for string literals
-  COUCHBASE
+  DEFAULT(false), // double quotes for string literals
+  ANSI_QUOTES(true), // double quotes for identifiers, single quotes for string literals
+
+  CASSANDRA(true),
+  CLICKHOUSE(true),
+  COUCHBASE(false),
+  GEODE(true),
+  INFLUXDB(true);
+
+  private final boolean ansiQuotes;
+
+  SqlDialect(boolean ansiQuotes) {
+    this.ansiQuotes = ansiQuotes;
+  }
+
+  /**
+   * Returns {@code true} if the dialect uses double quotes for identifiers (ANSI SQL standard),
+   * {@code false} if double quotes are used for string literals.
+   */
+  boolean ansiQuotes() {
+    return ansiQuotes;
+  }
 }

instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlStatementSanitizer.java

@@ -45,15 +45,15 @@ public SqlStatementInfo sanitize(@Nullable String statement, SqlDialect dialect)
     // cache growing too large
     // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/13180
     if (statement.length() > LARGE_STATEMENT_THRESHOLD) {
-      return sanitizeImpl(statement, dialect);
+      return sanitizeImpl(statement, dialect.ansiQuotes());
     }
     return sqlToStatementInfoCache.computeIfAbsent(
-        CacheKey.create(statement, dialect), k -> sanitizeImpl(statement, dialect));
+        CacheKey.create(statement, dialect), k -> sanitizeImpl(statement, dialect.ansiQuotes()));
   }
 
-  private static SqlStatementInfo sanitizeImpl(String statement, SqlDialect dialect) {
+  private static SqlStatementInfo sanitizeImpl(String statement, boolean ansiQuotes) {
     supportability.incrementCounter(SQL_STATEMENT_SANITIZER_CACHE_MISS);
-    return AutoSqlSanitizer.sanitize(statement, dialect);
+    return AutoSqlSanitizer.sanitize(statement, ansiQuotes);
   }
 
   // visible for tests

instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/db/SqlStatementSanitizerUtil.java

@@ -5,22 +5,45 @@
 
 package io.opentelemetry.instrumentation.api.incubator.semconv.db;
 
+import io.opentelemetry.instrumentation.api.incubator.semconv.db.SqlStatementSanitizer.CacheKey;
 import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
 import io.opentelemetry.instrumentation.api.internal.InstrumenterContext;
+import io.opentelemetry.semconv.DbAttributes;
+import java.util.Arrays;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 
 /**
  * Helper class for sanitizing sql that keeps sanitization results in {@link InstrumenterContext} so
  * that each statement would be sanitized only once for given {@link Instrumenter} call.
  */
 class SqlStatementSanitizerUtil {
   private static final SqlStatementSanitizer sanitizer = SqlStatementSanitizer.create(true);
+  private static final Set<String> dbsWithAnsiQuotes =
+      new HashSet<>(
+          Arrays.asList(
+              DbAttributes.DbSystemNameValues.POSTGRESQL,
+              "oracle",
+              "h2",
+              "hsqldb",
+              "db2",
+              "derby",
+              "hanadb"));
 
-  static SqlStatementInfo sanitize(String queryText) {
-    Map<String, SqlStatementInfo> map =
+  static SqlStatementInfo sanitize(String queryText, SqlDialect dialect) {
+    Map<CacheKey, SqlStatementInfo> map =
         InstrumenterContext.computeIfAbsent("sanitized-sql-map", unused -> new HashMap<>());
-    return map.computeIfAbsent(queryText, sanitizer::sanitize);
+    return map.computeIfAbsent(
+        CacheKey.create(queryText, dialect),
+        key -> sanitizer.sanitize(key.getStatement(), key.getDialect()));
+  }
+
+  static SqlDialect getDialect(String dbSystem, boolean ansiQuotes) {
+    return ansiQuotes || dbsWithAnsiQuotes.contains(dbSystem)
+        ? SqlDialect.ANSI_QUOTES
+        : SqlDialect.DEFAULT;
   }
 
   private SqlStatementSanitizerUtil() {}

instrumentation-api-incubator/src/main/jflex/SqlSanitizer.jflex

@@ -39,9 +39,9 @@ POSTGRE_PARAM_MARKER = "$"[0-9]*
 WHITESPACE           = [ \t\r\n]+
 
 %{
-  static SqlStatementInfo sanitize(String statement, SqlDialect dialect) {
+  static SqlStatementInfo sanitize(String statement, boolean ansiQuotes) {
     AutoSqlSanitizer sanitizer = new AutoSqlSanitizer(new java.io.StringReader(statement));
-    sanitizer.dialect = dialect;
+    sanitizer.ansiQuotes = ansiQuotes;
     try {
       while (!sanitizer.yyatEOF()) {
         int token = sanitizer.yylex();
@@ -114,7 +114,7 @@ WHITESPACE           = [ \t\r\n]+
   private boolean insideComment = false;
   private Operation operation = NoOp.INSTANCE;
   private boolean extractionDone = false;
-  private SqlDialect dialect;
+  private boolean ansiQuotes; // whether double quotes are used for quoting identifiers or string literals
 
   private void setOperation(Operation operation) {
     if (this.operation == NoOp.INSTANCE) {
@@ -534,13 +534,13 @@ WHITESPACE           = [ \t\r\n]+
       }
 
   {DOUBLE_QUOTED_STR} {
-          if (dialect == SqlDialect.COUCHBASE) {
-            builder.append('?');
-          } else {
+          if (ansiQuotes) {
             if (!insideComment && !extractionDone) {
               extractionDone = operation.handleIdentifier();
             }
             appendCurrentFragment();
+          } else {
+            builder.append('?');
           }
           if (isOverLimit()) return YYEOF;
       }