Fix clickhouse query failing with syntax error with agent (#13020)
This commit is contained in:
parent
8c15a7aed6
commit
dbc89b3edc
|
@ -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() {}
|
||||
}
|
|
@ -14,6 +14,7 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
|
|||
|
||||
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 class ClickHouseClientInstrumentation implements TypeInstrumentation {
|
|||
.getServer()
|
||||
.getDatabase()
|
||||
.orElse(ClickHouseDefaults.DATABASE.getDefaultValue().toString()),
|
||||
clickHouseRequest.getPreparedQuery().getOriginalQuery());
|
||||
ClickHouseRequestAccess.getQuery(clickHouseRequest));
|
||||
|
||||
return ClickHouseScope.start(parentContext, request);
|
||||
}
|
||||
|
|
|
@ -10,15 +10,27 @@ import static java.util.Collections.singletonList;
|
|||
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());
|
||||
|
|
|
@ -33,6 +33,7 @@ import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
|
|||
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 @@ class ClickHouseClientTest {
|
|||
"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(
|
||||
|
|
Loading…
Reference in New Issue