From 421c5e6476b57d52f6e1b2fa8d156143ca89f0d8 Mon Sep 17 00:00:00 2001 From: mschielmann Date: Mon, 3 Jan 2022 12:05:44 +0100 Subject: [PATCH] #2083 improvements (#2091) * #2083 improvements * removed known span names to keep backward compatibility --- .../instrument/jdbc/SpanNameProvider.java | 50 +++++++ .../jdbc/TraceListenerStrategy.java | 20 +-- .../instrument/jdbc/SpanNameProviderTest.java | 127 ++++++++++++++++++ 3 files changed, 179 insertions(+), 18 deletions(-) create mode 100644 spring-cloud-sleuth-instrumentation/src/main/java/org/springframework/cloud/sleuth/instrument/jdbc/SpanNameProvider.java create mode 100644 spring-cloud-sleuth-instrumentation/src/test/java/org/springframework/cloud/sleuth/instrument/jdbc/SpanNameProviderTest.java diff --git a/spring-cloud-sleuth-instrumentation/src/main/java/org/springframework/cloud/sleuth/instrument/jdbc/SpanNameProvider.java b/spring-cloud-sleuth-instrumentation/src/main/java/org/springframework/cloud/sleuth/instrument/jdbc/SpanNameProvider.java new file mode 100644 index 000000000..e45b9ae9f --- /dev/null +++ b/spring-cloud-sleuth-instrumentation/src/main/java/org/springframework/cloud/sleuth/instrument/jdbc/SpanNameProvider.java @@ -0,0 +1,50 @@ +/* + * Copyright 2013-2021 the original author or authors. + * + * Licensed 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 + * + * https://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.springframework.cloud.sleuth.instrument.jdbc; + +import java.util.Locale; +import java.util.Objects; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import static java.util.regex.Pattern.compile; + +class SpanNameProvider { + + private static final String DEFAULT_SPAN_NAME = "query"; + private static final Pattern PATTERN_MATCHING_FIRST_WORD_OF_SQL = compile("^([a-zA-Z]+)[^a-zA-Z]?.*$"); + + String getSpanNameFor(String sql) { + String spanName = DEFAULT_SPAN_NAME; + if (!Objects.isNull(sql)) { + spanName = getSpanNameForNonNull(sql); + } + + return spanName; + } + + private String getSpanNameForNonNull(String sql) { + String spanName = DEFAULT_SPAN_NAME; + Matcher matcher = PATTERN_MATCHING_FIRST_WORD_OF_SQL.matcher(sql); + + if (matcher.matches()) { + spanName = matcher.group(1).toLowerCase(Locale.ROOT); + } + + return spanName; + } +} diff --git a/spring-cloud-sleuth-instrumentation/src/main/java/org/springframework/cloud/sleuth/instrument/jdbc/TraceListenerStrategy.java b/spring-cloud-sleuth-instrumentation/src/main/java/org/springframework/cloud/sleuth/instrument/jdbc/TraceListenerStrategy.java index a53dd82e8..88a63bace 100644 --- a/spring-cloud-sleuth-instrumentation/src/main/java/org/springframework/cloud/sleuth/instrument/jdbc/TraceListenerStrategy.java +++ b/spring-cloud-sleuth-instrumentation/src/main/java/org/springframework/cloud/sleuth/instrument/jdbc/TraceListenerStrategy.java @@ -20,9 +20,7 @@ import java.net.URI; import java.sql.Connection; import java.sql.ResultSet; import java.sql.Statement; -import java.util.Arrays; import java.util.List; -import java.util.Locale; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Matcher; @@ -74,9 +72,7 @@ class TraceListenerStrategy { // string. private static final Pattern URL_SERVICE_NAME_FINDER = Pattern.compile("sleuthServiceName=(.*?)(?:&|$)"); - private static final String DEFAULT_SPAN_NAME = "query"; - - private static final List KNOWN_SPAN_NAMES = Arrays.asList("select", "insert", "update", "delete"); + private static final SpanNameProvider SPAN_NAME_PROVIDER = new SpanNameProvider(); private final Map openConnections = new ConcurrentHashMap<>(); @@ -235,7 +231,7 @@ class TraceListenerStrategy { SpanAndScope statementSpan = statementInfo.span; if (statementSpan != null) { AssertingSpan.of(SleuthJdbcSpan.JDBC_QUERY_SPAN, statementSpan.getSpan()) - .tag(SleuthJdbcSpan.QueryTags.QUERY, sql).name(spanName(sql)); + .tag(SleuthJdbcSpan.QueryTags.QUERY, sql).name(SPAN_NAME_PROVIDER.getSpanNameFor(sql)); if (t != null) { statementSpan.getSpan().error(t); } @@ -433,18 +429,6 @@ class TraceListenerStrategy { } } - private String spanName(String sql) { - String lowercaseSql = sql.toLowerCase(Locale.ROOT); - String spanName = DEFAULT_SPAN_NAME; - for (String spanNameCandidate : KNOWN_SPAN_NAMES) { - if (lowercaseSql.startsWith(spanNameCandidate)) { - spanName = spanNameCandidate; - break; - } - } - return spanName; - } - /** * This attempts to get the ip and port from the JDBC URL. Ex. localhost and 5555 from * {@code diff --git a/spring-cloud-sleuth-instrumentation/src/test/java/org/springframework/cloud/sleuth/instrument/jdbc/SpanNameProviderTest.java b/spring-cloud-sleuth-instrumentation/src/test/java/org/springframework/cloud/sleuth/instrument/jdbc/SpanNameProviderTest.java new file mode 100644 index 000000000..599a0ea18 --- /dev/null +++ b/spring-cloud-sleuth-instrumentation/src/test/java/org/springframework/cloud/sleuth/instrument/jdbc/SpanNameProviderTest.java @@ -0,0 +1,127 @@ +/* + * Copyright 2013-2021 the original author or authors. + * + * Licensed 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 + * + * https://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.springframework.cloud.sleuth.instrument.jdbc; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class SpanNameProviderTest { + + private static final String DEFAULT_SPAN_NAME = "query"; + private static final String SPAN_NAME_FOR_SELECTS = "select"; + private static final String SPAN_NAME_FOR_UPDATES = "update"; + private static final String SPAN_NAME_FOR_INSERTS = "insert"; + private static final String SPAN_NAME_FOR_DELETES = "delete"; + + @Test + public void should_return_default_on_null_input() { + SpanNameProvider provider = new SpanNameProvider(); + String sql = null; + + @SuppressWarnings("ConstantConditions") + String result = provider.getSpanNameFor(sql); + + assertThat(result).isEqualTo(DEFAULT_SPAN_NAME); + } + + @Test + public void should_return_default_on_empty_input() { + SpanNameProvider provider = new SpanNameProvider(); + String sql = ""; + + String result = provider.getSpanNameFor(sql); + + assertThat(result).isEqualTo(DEFAULT_SPAN_NAME); + } + + @Test + public void should_return_word_select_on_input_starting_with_word_select() { + SpanNameProvider provider = new SpanNameProvider(); + String sql = "SELECT * FROM test_table;"; + + String result = provider.getSpanNameFor(sql); + + assertThat(result).isEqualTo(SPAN_NAME_FOR_SELECTS); + } + + @Test + public void should_return_word_update_on_input_starting_with_word_update() { + SpanNameProvider provider = new SpanNameProvider(); + String sql = "UPDATE test_table SET foo = 'bar';"; + + String result = provider.getSpanNameFor(sql); + + assertThat(result).isEqualTo(SPAN_NAME_FOR_UPDATES); + } + + @Test + public void should_return_word_insert_on_input_starting_with_word_insert() { + SpanNameProvider provider = new SpanNameProvider(); + String sql = "INSERT INTO test_table (foo) VALUES ('bar');"; + + String result = provider.getSpanNameFor(sql); + + assertThat(result).isEqualTo(SPAN_NAME_FOR_INSERTS); + } + + @Test + public void should_return_word_delete_on_input_starting_with_word_delete() { + SpanNameProvider provider = new SpanNameProvider(); + String sql = "DELETE FROM test_table;"; + + String result = provider.getSpanNameFor(sql); + + assertThat(result).isEqualTo(SPAN_NAME_FOR_DELETES); + } + + @Test + public void should_be_case_insensitive() { + SpanNameProvider provider = new SpanNameProvider(); + String lowerCaseSql = "select * from test_table;"; + String mixedCaseSql = "SelECT * FRom TeSt_TaBLE;"; + String upperCaseSql = "SELECT * FROM TEST_TABLE;"; + + String resultForLowerCaseInput = provider.getSpanNameFor(lowerCaseSql); + String resultForMixedCaseInput = provider.getSpanNameFor(mixedCaseSql); + String resultForUpperCaseInput = provider.getSpanNameFor(upperCaseSql); + + assertThat(resultForLowerCaseInput).isEqualTo(SPAN_NAME_FOR_SELECTS); + assertThat(resultForMixedCaseInput).isEqualTo(SPAN_NAME_FOR_SELECTS); + assertThat(resultForUpperCaseInput).isEqualTo(SPAN_NAME_FOR_SELECTS); + } + + @Test + public void should_handle_sql_without_redundant_spaces() { + SpanNameProvider provider = new SpanNameProvider(); + String sql = "select*from test_table;"; + + String result = provider.getSpanNameFor(sql); + + assertThat(result).isEqualTo(SPAN_NAME_FOR_SELECTS); + } + + @Test + public void should_return_lower_case_input_for_input_containing_only_one_word() { + SpanNameProvider provider = new SpanNameProvider(); + String sql = "BEGIN"; + String expectedResult = "begin"; + + String result = provider.getSpanNameFor(sql); + + assertThat(result).isEqualTo(expectedResult); + } +}