diff --git a/CHANGES.md b/CHANGES.md index ef6a81a3e4..3a9514fdc6 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -24,6 +24,7 @@ Release Notes. * Add Elasticsearch Java client (co.elastic.clients:elasticsearch-java) plugin for 7.16.x-9.x. * Only publish `apm-application-toolkit` modules to Maven Central. Agent and plugins are distributed via download package and Docker images. * Add unified release script (`tools/releasing/release.sh`) with two-step flow: `prepare-vote` and `vote-passed`. +* Fix an issue where `JDBCPluginConfig.Plugin.JDBC.SQL_BODY_MAX_LENGTH` was not honored by clickhouse-0.3.1 and clickhouse-0.3.2.x plugins. All issues and pull requests are [here](https://github.com/apache/skywalking/milestone/249?closed=1) diff --git a/apm-sniffer/apm-sdk-plugin/clickhouse-0.3.1-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/clickhouse/ClickHouseStatementTracingWrapper.java b/apm-sniffer/apm-sdk-plugin/clickhouse-0.3.1-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/clickhouse/ClickHouseStatementTracingWrapper.java index 6f2e9bcaa0..2599fb3455 100644 --- a/apm-sniffer/apm-sdk-plugin/clickhouse-0.3.1-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/clickhouse/ClickHouseStatementTracingWrapper.java +++ b/apm-sniffer/apm-sdk-plugin/clickhouse-0.3.1-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/clickhouse/ClickHouseStatementTracingWrapper.java @@ -23,6 +23,7 @@ import org.apache.skywalking.apm.agent.core.context.tag.Tags; import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan; import org.apache.skywalking.apm.agent.core.context.trace.SpanLayer; +import org.apache.skywalking.apm.plugin.jdbc.SqlBodyUtil; import org.apache.skywalking.apm.plugin.jdbc.trace.ConnectionInfo; /** @@ -37,7 +38,7 @@ public static T of(ConnectionInfo connectionInfo, String methodName, String try { Tags.DB_TYPE.set(span, connectionInfo.getDBType()); Tags.DB_INSTANCE.set(span, connectionInfo.getDatabaseName()); - Tags.DB_STATEMENT.set(span, sql); + Tags.DB_STATEMENT.set(span, SqlBodyUtil.limitSqlBodySize(sql)); span.setComponent(connectionInfo.getComponent()); SpanLayer.asDB(span); return supplier.get(); diff --git a/apm-sniffer/apm-sdk-plugin/clickhouse-0.3.1-plugin/src/test/java/org/apache/skywalking/apm/plugin/jdbc/clickhouse/ClickHouseStatementTracingWrapperTest.java b/apm-sniffer/apm-sdk-plugin/clickhouse-0.3.1-plugin/src/test/java/org/apache/skywalking/apm/plugin/jdbc/clickhouse/ClickHouseStatementTracingWrapperTest.java new file mode 100644 index 0000000000..3c330478fc --- /dev/null +++ b/apm-sniffer/apm-sdk-plugin/clickhouse-0.3.1-plugin/src/test/java/org/apache/skywalking/apm/plugin/jdbc/clickhouse/ClickHouseStatementTracingWrapperTest.java @@ -0,0 +1,117 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.apm.plugin.jdbc.clickhouse; + +import java.util.List; +import org.apache.skywalking.apm.agent.core.context.trace.AbstractTracingSpan; +import org.apache.skywalking.apm.agent.core.context.trace.TraceSegment; +import org.apache.skywalking.apm.agent.core.context.util.TagValuePair; +import org.apache.skywalking.apm.agent.test.helper.SegmentHelper; +import org.apache.skywalking.apm.agent.test.helper.SpanHelper; +import org.apache.skywalking.apm.agent.test.tools.AgentServiceRule; +import org.apache.skywalking.apm.agent.test.tools.SegmentStorage; +import org.apache.skywalking.apm.agent.test.tools.SegmentStoragePoint; +import org.apache.skywalking.apm.agent.test.tools.TracingSegmentRunner; +import org.apache.skywalking.apm.network.trace.component.ComponentsDefine; +import org.apache.skywalking.apm.plugin.jdbc.JDBCPluginConfig; +import org.apache.skywalking.apm.plugin.jdbc.trace.ConnectionInfo; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; + +/** + * Verify that {@link ClickHouseStatementTracingWrapper} truncates the SQL body + * recorded on the exit span according to {@link JDBCPluginConfig.Plugin.JDBC#SQL_BODY_MAX_LENGTH}. + */ +@RunWith(TracingSegmentRunner.class) +public class ClickHouseStatementTracingWrapperTest { + + @SegmentStoragePoint + private SegmentStorage segmentStorage; + + @Rule + public AgentServiceRule serviceRule = new AgentServiceRule(); + + private ConnectionInfo connectionInfo; + + private int originalLimit; + + @Before + public void setUp() { + connectionInfo = new ConnectionInfo( + ComponentsDefine.CLICKHOUSE_JDBC_DRIVER, "ClickHouse", "127.0.0.1", 8123, "default"); + originalLimit = JDBCPluginConfig.Plugin.JDBC.SQL_BODY_MAX_LENGTH; + } + + @After + public void tearDown() { + JDBCPluginConfig.Plugin.JDBC.SQL_BODY_MAX_LENGTH = originalLimit; + } + + @Test + public void shortSqlIsRecordedAsIs() throws Exception { + JDBCPluginConfig.Plugin.JDBC.SQL_BODY_MAX_LENGTH = 2048; + String sql = "SELECT 1"; + + ClickHouseStatementTracingWrapper.of(connectionInfo, "execute", sql, () -> Boolean.TRUE); + + assertThat(dbStatementTagValue(), is(sql)); + } + + @Test + public void longSqlIsTruncatedToConfiguredLimit() throws Exception { + JDBCPluginConfig.Plugin.JDBC.SQL_BODY_MAX_LENGTH = 16; + // Longer than the limit, so the helper must truncate to first 16 chars and append "..." + String sql = "SELECT * FROM table_with_many_cols"; + + ClickHouseStatementTracingWrapper.of(connectionInfo, "execute", sql, () -> Boolean.TRUE); + + assertThat(dbStatementTagValue(), is(sql.substring(0, 16) + "...")); + } + + @Test + public void negativeLimitDisablesTruncation() throws Exception { + JDBCPluginConfig.Plugin.JDBC.SQL_BODY_MAX_LENGTH = -1; + StringBuilder builder = new StringBuilder("SELECT "); + for (int i = 0; i < 1000; i++) { + builder.append("a, "); + } + builder.append("a FROM t"); + String sql = builder.toString(); + + ClickHouseStatementTracingWrapper.of(connectionInfo, "execute", sql, () -> Boolean.TRUE); + + assertThat(dbStatementTagValue(), is(sql)); + } + + private String dbStatementTagValue() { + List traceSegments = segmentStorage.getTraceSegments(); + assertThat(traceSegments.size(), is(1)); + List spans = SegmentHelper.getSpans(traceSegments.get(0)); + assertThat(spans.size(), is(1)); + List tags = SpanHelper.getTags(spans.get(0)); + // tag order: db.type, db.instance, db.statement + return (String) tags.get(2).getValue(); + } +} diff --git a/apm-sniffer/apm-sdk-plugin/clickhouse-0.3.2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/clickhouse/v32/ClickHousePrepareStatementTracing.java b/apm-sniffer/apm-sdk-plugin/clickhouse-0.3.2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/clickhouse/v32/ClickHousePrepareStatementTracing.java index 292dd413a7..5c94003cf2 100644 --- a/apm-sniffer/apm-sdk-plugin/clickhouse-0.3.2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/clickhouse/v32/ClickHousePrepareStatementTracing.java +++ b/apm-sniffer/apm-sdk-plugin/clickhouse-0.3.2.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/jdbc/clickhouse/v32/ClickHousePrepareStatementTracing.java @@ -22,6 +22,7 @@ import org.apache.skywalking.apm.agent.core.context.tag.Tags; import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan; import org.apache.skywalking.apm.agent.core.context.trace.SpanLayer; +import org.apache.skywalking.apm.plugin.jdbc.SqlBodyUtil; import org.apache.skywalking.apm.plugin.jdbc.trace.ConnectionInfo; import java.sql.SQLException; @@ -39,7 +40,7 @@ public static T of(ConnectionInfo connectionInfo, String methodName, String try { Tags.DB_TYPE.set(span, connectionInfo.getDBType()); Tags.DB_INSTANCE.set(span, connectionInfo.getDatabaseName()); - Tags.DB_STATEMENT.set(span, sql); + Tags.DB_STATEMENT.set(span, SqlBodyUtil.limitSqlBodySize(sql)); span.setComponent(connectionInfo.getComponent()); SpanLayer.asDB(span); return supplier.get(); diff --git a/apm-sniffer/apm-sdk-plugin/clickhouse-0.3.2.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/jdbc/clickhouse/ClickHousePrepareStatementTracingTest.java b/apm-sniffer/apm-sdk-plugin/clickhouse-0.3.2.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/jdbc/clickhouse/ClickHousePrepareStatementTracingTest.java new file mode 100644 index 0000000000..49d2de6ff2 --- /dev/null +++ b/apm-sniffer/apm-sdk-plugin/clickhouse-0.3.2.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/jdbc/clickhouse/ClickHousePrepareStatementTracingTest.java @@ -0,0 +1,118 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.apm.plugin.jdbc.clickhouse; + +import java.util.List; +import org.apache.skywalking.apm.agent.core.context.trace.AbstractTracingSpan; +import org.apache.skywalking.apm.agent.core.context.trace.TraceSegment; +import org.apache.skywalking.apm.agent.core.context.util.TagValuePair; +import org.apache.skywalking.apm.agent.test.helper.SegmentHelper; +import org.apache.skywalking.apm.agent.test.helper.SpanHelper; +import org.apache.skywalking.apm.agent.test.tools.AgentServiceRule; +import org.apache.skywalking.apm.agent.test.tools.SegmentStorage; +import org.apache.skywalking.apm.agent.test.tools.SegmentStoragePoint; +import org.apache.skywalking.apm.agent.test.tools.TracingSegmentRunner; +import org.apache.skywalking.apm.network.trace.component.ComponentsDefine; +import org.apache.skywalking.apm.plugin.jdbc.JDBCPluginConfig; +import org.apache.skywalking.apm.plugin.jdbc.clickhouse.v32.ClickHousePrepareStatementTracing; +import org.apache.skywalking.apm.plugin.jdbc.trace.ConnectionInfo; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; + +/** + * Verify that {@link ClickHousePrepareStatementTracing} truncates the SQL body + * recorded on the exit span according to {@link JDBCPluginConfig.Plugin.JDBC#SQL_BODY_MAX_LENGTH}. + */ +@RunWith(TracingSegmentRunner.class) +public class ClickHousePrepareStatementTracingTest { + + @SegmentStoragePoint + private SegmentStorage segmentStorage; + + @Rule + public AgentServiceRule serviceRule = new AgentServiceRule(); + + private ConnectionInfo connectionInfo; + + private int originalLimit; + + @Before + public void setUp() { + connectionInfo = new ConnectionInfo( + ComponentsDefine.CLICKHOUSE_JDBC_DRIVER, "ClickHouse", "127.0.0.1", 8123, "test"); + originalLimit = JDBCPluginConfig.Plugin.JDBC.SQL_BODY_MAX_LENGTH; + } + + @After + public void tearDown() { + JDBCPluginConfig.Plugin.JDBC.SQL_BODY_MAX_LENGTH = originalLimit; + } + + @Test + public void shortSqlIsRecordedAsIs() throws Exception { + JDBCPluginConfig.Plugin.JDBC.SQL_BODY_MAX_LENGTH = 2048; + String sql = "SELECT 1"; + + ClickHousePrepareStatementTracing.of(connectionInfo, "execute", sql, () -> Boolean.TRUE); + + assertThat(dbStatementTagValue(), is(sql)); + } + + @Test + public void longSqlIsTruncatedToConfiguredLimit() throws Exception { + JDBCPluginConfig.Plugin.JDBC.SQL_BODY_MAX_LENGTH = 16; + // Longer than the configured limit, so the helper must truncate to the first 16 chars and append "..." + String sql = "SELECT * FROM table_with_many_cols"; + + ClickHousePrepareStatementTracing.of(connectionInfo, "execute", sql, () -> Boolean.TRUE); + + assertThat(dbStatementTagValue(), is(sql.substring(0, 16) + "...")); + } + + @Test + public void negativeLimitDisablesTruncation() throws Exception { + JDBCPluginConfig.Plugin.JDBC.SQL_BODY_MAX_LENGTH = -1; + StringBuilder builder = new StringBuilder("SELECT "); + for (int i = 0; i < 1000; i++) { + builder.append("a, "); + } + builder.append("a FROM t"); + String sql = builder.toString(); + + ClickHousePrepareStatementTracing.of(connectionInfo, "execute", sql, () -> Boolean.TRUE); + + assertThat(dbStatementTagValue(), is(sql)); + } + + private String dbStatementTagValue() { + List traceSegments = segmentStorage.getTraceSegments(); + assertThat(traceSegments.size(), is(1)); + List spans = SegmentHelper.getSpans(traceSegments.get(0)); + assertThat(spans.size(), is(1)); + List tags = SpanHelper.getTags(spans.get(0)); + // tag order: db.type, db.instance, db.statement + return (String) tags.get(2).getValue(); + } +}