Fix clickhouse query failing with syntax error with agent (#13020)

open-telemetry · Jan 9, 2025 · dbc89b3 · dbc89b3
1 parent 8c15a7a
commit dbc89b3
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 2 deletions.
diff --git a/...use-client-0.5/javaagent/src/main/java/com/clickhouse/client/ClickHouseRequestAccess.java b/...use-client-0.5/javaagent/src/main/java/com/clickhouse/client/ClickHouseRequestAccess.java
@@ -0,0 +1,16 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package com.clickhouse.client;
+
+// helper class for accessing package private members in com.clickhouse.client package
+public final class ClickHouseRequestAccess {
+
+  public static String getQuery(ClickHouseRequest<?> clickHouseRequest) {
+    return clickHouseRequest.getQuery();
+  }
+
+  private ClickHouseRequestAccess() {}
+}
diff --git a/...o/opentelemetry/javaagent/instrumentation/clickhouse/ClickHouseClientInstrumentation.java b/...o/opentelemetry/javaagent/instrumentation/clickhouse/ClickHouseClientInstrumentation.java
@@ -14,6 +14,7 @@
 
 import com.clickhouse.client.ClickHouseClient;
 import com.clickhouse.client.ClickHouseRequest;
+import com.clickhouse.client.ClickHouseRequestAccess;
 import com.clickhouse.client.config.ClickHouseDefaults;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.javaagent.bootstrap.CallDepth;
@@ -59,7 +60,7 @@ public static ClickHouseScope onEnter(
                   .getServer()
                   .getDatabase()
                   .orElse(ClickHouseDefaults.DATABASE.getDefaultValue().toString()),
-              clickHouseRequest.getPreparedQuery().getOriginalQuery());
+              ClickHouseRequestAccess.getQuery(clickHouseRequest));
 
       return ClickHouseScope.start(parentContext, request);
     }

diff --git a/...o/opentelemetry/javaagent/instrumentation/clickhouse/ClickHouseInstrumentationModule.java b/...o/opentelemetry/javaagent/instrumentation/clickhouse/ClickHouseInstrumentationModule.java
@@ -10,15 +10,27 @@
 import com.google.auto.service.AutoService;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
+import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
 import java.util.List;
 
 @AutoService(InstrumentationModule.class)
-public class ClickHouseInstrumentationModule extends InstrumentationModule {
+public class ClickHouseInstrumentationModule extends InstrumentationModule
+    implements ExperimentalInstrumentationModule {
 
   public ClickHouseInstrumentationModule() {
     super("clickhouse-client", "clickhouse-client-0.5", "clickhouse");
   }
 
+  @Override
+  public boolean isHelperClass(String className) {
+    return "com.clickhouse.client.ClickHouseRequestAccess".equals(className);
+  }
+
+  @Override
+  public List<String> injectedClassNames() {
+    return singletonList("com.clickhouse.client.ClickHouseRequestAccess");
+  }
+
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return singletonList(new ClickHouseClientInstrumentation());

diff --git a/...test/java/io/opentelemetry/javaagent/instrumentation/clickhouse/ClickHouseClientTest.java b/...test/java/io/opentelemetry/javaagent/instrumentation/clickhouse/ClickHouseClientTest.java
@@ -33,6 +33,7 @@
 import io.opentelemetry.sdk.testing.assertj.AttributeAssertion;
 import io.opentelemetry.sdk.trace.data.StatusData;
 import io.opentelemetry.semconv.incubating.DbIncubatingAttributes;
+import java.time.Instant;
 import java.util.List;
 import java.util.concurrent.CompletableFuture;
 import org.junit.jupiter.api.AfterAll;
@@ -338,6 +339,41 @@ void testParameterizedQueryInput() throws ClickHouseException {
                                 "select * from " + tableName + " where s=:val", "SELECT"))));
   }
 
+  // regression test for
+  // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/13019
+  // {s:String} used in the query really a syntax error, should be {s: String}. This test verifies
+  // that this syntax error isn't detected when running with the agent as it is also ignored when
+  // running without the agent.
+  @Test
+  void testPlaceholderQueryInput() throws Exception {
+    ClickHouseRequest<?> request =
+        client.read(server).format(ClickHouseFormat.RowBinaryWithNamesAndTypes);
+    testing.runWithSpan(
+        "parent",
+        () -> {
+          ClickHouseResponse response =
+              request
+                  // {s:String} is really a syntax error should be {s: String}
+                  .query("select * from " + tableName + " where s={s:String}")
+                  .settings(ImmutableMap.of("param_s", "" + Instant.now().getEpochSecond()))
+                  .execute()
+                  .get();
+          response.close();
+        });
+
+    testing.waitAndAssertTraces(
+        trace ->
+            trace.hasSpansSatisfyingExactly(
+                span -> span.hasName("parent").hasNoParent().hasAttributes(Attributes.empty()),
+                span ->
+                    span.hasName("SELECT " + dbName)
+                        .hasKind(SpanKind.CLIENT)
+                        .hasParent(trace.getSpan(0))
+                        .hasAttributesSatisfyingExactly(
+                            attributeAssertions(
+                                "select * from " + tableName + " where s={s:String}", "SELECT"))));
+  }
+
   @SuppressWarnings("deprecation") // using deprecated semconv
   private static List<AttributeAssertion> attributeAssertions(String statement, String operation) {
     return asList(