From 5e65a48fae44bfdcd79e946f399617e0b2754671 Mon Sep 17 00:00:00 2001 From: wolframhaussig <13997737+wolframhaussig@users.noreply.github.com> Date: Wed, 26 Jun 2019 14:57:57 +0200 Subject: [PATCH 01/14] further SQL parsing improvement (#676) support for CALL using JDBC escape syntax support for MERGE MERGE support also contains support for DB links --- .../apm/agent/jdbc/signature/Scanner.java | 29 ++++++++++++-- .../agent/jdbc/signature/SignatureParser.java | 40 +++++++++++++++++++ .../src/test/resources/signature_tests.json | 32 +++++++++++++++ 3 files changed, 97 insertions(+), 4 deletions(-) diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/Scanner.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/Scanner.java index 7ef39cfefc..ef978e0394 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/Scanner.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/Scanner.java @@ -68,8 +68,10 @@ public boolean scanToken(Token token) { } return false; } - public Token scan() { + return scan(true); + } + public Token scan(boolean skipDoubleQuotes) { if (!hasNext()) { return Token.EOF; } @@ -98,10 +100,17 @@ public Token scan() { // string literal delimiter by default, // but we assume standard SQL and treat // it as a identifier delimiter. - return scanQuotedIdentifier('"'); + if(skipDoubleQuotes) { + return scanQuotedIdentifier('"'); + }else { + return Token.DQUOT; + } case '[': // T-SQL bracket-quoted identifier return scanQuotedIdentifier(']'); + case '{': + // JDBC escapes + return scan(skipDoubleQuotes); case '`': // MySQL-style backtick-quoted identifier return scanQuotedIdentifier('`'); @@ -125,6 +134,14 @@ public Token scan() { return Token.OTHER; case '.': return Token.PERIOD; + case '@': + return Token.AT; + case '?': + //skip JDBC variables + return scan(skipDoubleQuotes); + case '=': + //skip equals as in ? = call function() + return scan(skipDoubleQuotes); case '$': if (!hasNext()) { return Token.OTHER; @@ -360,6 +377,8 @@ public enum Token { PERIOD, // . LPAREN, // ( RPAREN, // ) + AT, // @ + DQUOT, // " AS, CALL, @@ -373,7 +392,9 @@ public enum Token { SET, TABLE, TRUNCATE, // Cassandra/CQL-specific - UPDATE; + UPDATE, + MERGE, + USING; private static final Token[] EMPTY = {}; private static final Token[][] KEYWORDS_BY_LENGTH = { @@ -382,7 +403,7 @@ public enum Token { {AS, OR}, {SET}, {CALL, FROM, INTO}, - {TABLE}, + {TABLE, MERGE, USING}, {DELETE, INSERT, SELECT, UPDATE}, {REPLACE}, {TRUNCATE} diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java index 7dd157933c..8769bb4924 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java @@ -32,6 +32,7 @@ import static co.elastic.apm.agent.jdbc.signature.Scanner.Token.IDENT; import static co.elastic.apm.agent.jdbc.signature.Scanner.Token.LPAREN; import static co.elastic.apm.agent.jdbc.signature.Scanner.Token.RPAREN; +import static co.elastic.apm.agent.jdbc.signature.Scanner.Token.INTO; public class SignatureParser { @@ -160,6 +161,45 @@ private void parse(String query, StringBuilder signature) { } } return; + case MERGE: + signature.append("MERGE"); + scanner.scanToken(INTO); + if (scanner.scanToken(IDENT)) { + signature.append(' '); + scanner.appendCurrentTokenText(signature); + boolean connectedIdents=false; + boolean inQuotes=false; + for (Scanner.Token t = scanner.scan(false); t != EOF; t = scanner.scan(false)) { + switch (t) { + case IDENT: + //do not add tokens which are separated by a space + if(connectedIdents) { + scanner.appendCurrentTokenText(signature); + connectedIdents=false; + }else { + return; + } + break; + case PERIOD: + signature.append('.'); + connectedIdents=true; + break; + case AT: + signature.append('@'); + connectedIdents=true; + break; + case DQUOT: + signature.append('"'); + inQuotes=!inQuotes; + break; + case USING: + return; + default: + break; + } + } + } + return; default: query = query.trim(); final int indexOfWhitespace = query.indexOf(' '); diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/test/resources/signature_tests.json b/apm-agent-plugins/apm-jdbc-plugin/src/test/resources/signature_tests.json index 564fbbf814..1afa289bd8 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/test/resources/signature_tests.json +++ b/apm-agent-plugins/apm-jdbc-plugin/src/test/resources/signature_tests.json @@ -142,5 +142,37 @@ { "input": "CALL", "output": "CALL" + }, + { + "input": "MERGE INTO TEST USING (SELECT * FROM DUAL) SOURCE on (TEST.X=SOURCE.DUMMY) WHEN NOT MATCHED THEN INSERT VALUES(SOURCE.DUMMY)", + "output": "MERGE TEST" + }, + { + "input": "MERGE INTO TEST ALIAS USING (SELECT * FROM DUAL) SOURCE on (TEST.X=SOURCE.DUMMY) WHEN NOT MATCHED THEN INSERT VALUES(SOURCE.DUMMY)", + "output": "MERGE TEST" + }, + { + "input": "MERGE INTO SCHEMA.TEST USING (SELECT * FROM DUAL) SOURCE on (TEST.X=SOURCE.DUMMY) WHEN NOT MATCHED THEN INSERT VALUES(SOURCE.DUMMY)", + "output": "MERGE SCHEMA.TEST" + }, + { + "input": "MERGE INTO SCHEMA.TEST@DBLINK USING (SELECT * FROM DUAL) SOURCE on (TEST.X=SOURCE.DUMMY) WHEN NOT MATCHED THEN INSERT VALUES(SOURCE.DUMMY)", + "output": "MERGE SCHEMA.TEST@DBLINK" + }, + { + "input": "MERGE INTO SCHEMA.TEST@\"DBLINK.FQDN.COM\" USING (SELECT * FROM DUAL) SOURCE on (TEST.X=SOURCE.DUMMY) WHEN NOT MATCHED THEN INSERT VALUES(SOURCE.DUMMY)", + "output": "MERGE SCHEMA.TEST@\"DBLINK.FQDN.COM\"" + }, + { + "input": "{ ? = call FUNCTION() }", + "output": "CALL FUNCTION" + }, + { + "input": "{ call PROCEDURE() }", + "output": "CALL PROCEDURE" + }, + { + "input": "{call PROCEDURE() }", + "output": "CALL PROCEDURE" } ] From af23e5ddc0129d44ad15e86792644fa859f64528 Mon Sep 17 00:00:00 2001 From: wolframhaussig <13997737+wolframhaussig@users.noreply.github.com> Date: Thu, 27 Jun 2019 08:19:26 +0200 Subject: [PATCH 02/14] further SQL parsing improvement (#676) support DB links on CALL, DELETE, INSERT, SELECT, UPDATE MERGE now shows MERGE INTO instead of MERGE --- .../agent/jdbc/signature/SignatureParser.java | 106 +++++++++--------- .../src/test/resources/signature_tests.json | 50 ++++++++- 2 files changed, 101 insertions(+), 55 deletions(-) diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java index 8769bb4924..c786adca1d 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java @@ -80,21 +80,19 @@ public void querySignature(String query, StringBuilder signature, boolean prepar signatureCache.put(query, signature.toString()); } } - private void parse(String query, StringBuilder signature) { final Scanner.Token firstToken = scanner.scanWhile(Scanner.Token.COMMENT); switch (firstToken) { case CALL: signature.append("CALL"); - if (scanner.scanUntil(Scanner.Token.IDENT)) { - signature.append(' '); - scanner.appendCurrentTokenText(signature); + if(scanner.scanUntil(Scanner.Token.IDENT)) { + appendIdentifiers(signature); } return; case DELETE: signature.append("DELETE"); if (scanner.scanUntil(FROM) && scanner.scanUntil(Scanner.Token.IDENT)) { - signature.append(" FROM "); + signature.append(" FROM"); appendIdentifiers(signature); } return; @@ -102,7 +100,7 @@ private void parse(String query, StringBuilder signature) { case REPLACE: signature.append(firstToken.name()); if (scanner.scanUntil(Scanner.Token.INTO) && scanner.scanUntil(Scanner.Token.IDENT)) { - signature.append(" INTO "); + signature.append(" INTO"); appendIdentifiers(signature); } return; @@ -117,7 +115,7 @@ private void parse(String query, StringBuilder signature) { } else if (t == FROM) { if (level == 0) { if (scanner.scanToken(Scanner.Token.IDENT)) { - signature.append(" FROM "); + signature.append(" FROM"); appendIdentifiers(signature); } else { return; @@ -129,18 +127,21 @@ private void parse(String query, StringBuilder signature) { case UPDATE: signature.append("UPDATE"); // Scan for the table name - boolean hasPeriod = false, hasFirstPeriod = false; + boolean hasPeriod = false, hasFirstPeriod = false, inQuotes = false; if (scanner.scanToken(IDENT)) { signature.append(' '); scanner.appendCurrentTokenText(signature); - for (Scanner.Token t = scanner.scan(); t != EOF; t = scanner.scan()) { + for (Scanner.Token t = scanner.scan(false); t != EOF; t = scanner.scan(false)) { switch (t) { case IDENT: if (hasPeriod) { scanner.appendCurrentTokenText(signature); hasPeriod = false; } - if (!hasFirstPeriod) { + else if(inQuotes) { + scanner.appendCurrentTokenText(signature); + } + if (!hasFirstPeriod && !inQuotes) { // Some dialects allow option keywords before the table name // example: UPDATE IGNORE foo.bar signature.setLength(0); @@ -155,6 +156,15 @@ private void parse(String query, StringBuilder signature) { hasPeriod = true; signature.append('.'); break; + case AT: + hasFirstPeriod = true; + hasPeriod = true; + signature.append('@'); + break; + case DQUOT: + signature.append('"'); + inQuotes = !inQuotes; + break; default: return; } @@ -163,41 +173,9 @@ private void parse(String query, StringBuilder signature) { return; case MERGE: signature.append("MERGE"); - scanner.scanToken(INTO); - if (scanner.scanToken(IDENT)) { - signature.append(' '); - scanner.appendCurrentTokenText(signature); - boolean connectedIdents=false; - boolean inQuotes=false; - for (Scanner.Token t = scanner.scan(false); t != EOF; t = scanner.scan(false)) { - switch (t) { - case IDENT: - //do not add tokens which are separated by a space - if(connectedIdents) { - scanner.appendCurrentTokenText(signature); - connectedIdents=false; - }else { - return; - } - break; - case PERIOD: - signature.append('.'); - connectedIdents=true; - break; - case AT: - signature.append('@'); - connectedIdents=true; - break; - case DQUOT: - signature.append('"'); - inQuotes=!inQuotes; - break; - case USING: - return; - default: - break; - } - } + if(scanner.scanToken(INTO) && scanner.scanUntil(Scanner.Token.IDENT)) { + signature.append(" INTO"); + appendIdentifiers(signature); } return; default: @@ -208,10 +186,38 @@ private void parse(String query, StringBuilder signature) { } private void appendIdentifiers(StringBuilder signature) { - scanner.appendCurrentTokenText(signature); - while (scanner.scanToken(Scanner.Token.PERIOD) && scanner.scanToken(Scanner.Token.IDENT)) { - signature.append('.'); - scanner.appendCurrentTokenText(signature); - } + signature.append(' '); + scanner.appendCurrentTokenText(signature); + boolean connectedIdents = false; + boolean inQuotes = false; + for (Scanner.Token t = scanner.scan(false); t != EOF; t = scanner.scan(false)) { + switch (t) { + case IDENT: + // do not add tokens which are separated by a space + if (connectedIdents) { + scanner.appendCurrentTokenText(signature); + connectedIdents = false; + } else { + return; + } + break; + case PERIOD: + signature.append('.'); + connectedIdents = true; + break; + case AT: + signature.append('@'); + connectedIdents = true; + break; + case DQUOT: + signature.append('"'); + inQuotes = !inQuotes; + break; + case USING: + return; + default: + break; + } + } } } diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/test/resources/signature_tests.json b/apm-agent-plugins/apm-jdbc-plugin/src/test/resources/signature_tests.json index 1afa289bd8..eddd3c5630 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/test/resources/signature_tests.json +++ b/apm-agent-plugins/apm-jdbc-plugin/src/test/resources/signature_tests.json @@ -52,6 +52,10 @@ "input": "SELECT *,(SELECT COUNT(*) FROM table2 WHERE table2.field1 = table1.id) AS count FROM table1 WHERE table1.field1 = 'value'", "output": "SELECT FROM table1" }, + { + "input": "SELECT *,(SELECT COUNT(*) FROM table2 WHERE table2.field1 = table1.id) AS count FROM schema.table1@\"DBLINK.FQDN.COM@USER\" WHERE table1.field1 = 'value'", + "output": "SELECT FROM schema.table1@\"DBLINK.FQDN.COM@USER\"" + }, { "comment": "If the outermost select operates on derived tables, then we just return 'SELECT' (i.e. the fallback)", "input": "SELECT * FROM (SELECT foo FROM bar) AS foo_bar", @@ -65,6 +69,10 @@ "input": "UPDATE IGNORE foo.bar SET bar=1 WHERE baz=2", "output": "UPDATE foo.bar" }, + { + "input": "UPDATE IGNORE foo.bar@\"DBLINK.FQDN.COM@USER\" SET bar=1 WHERE baz=2", + "output": "UPDATE foo.bar@\"DBLINK.FQDN.COM@USER\"" + }, { "input": "UPDATE ONLY foo AS bar SET baz=1", "output": "UPDATE foo" @@ -77,6 +85,10 @@ "input": "INSERT LOW_PRIORITY IGNORE INTO foo.bar (col) VALUES(?)", "output": "INSERT INTO foo.bar" }, + { + "input": "INSERT LOW_PRIORITY IGNORE INTO schema.table1@\"DBLINK.FQDN.COM@USER\" (col) VALUES(?)", + "output": "INSERT INTO schema.table1@\"DBLINK.FQDN.COM@USER\"" + }, { "input": "CALL foo(bar, 123)", "output": "CALL foo" @@ -139,38 +151,66 @@ "input": "DELETE FROM", "output": "DELETE" }, + { + "input": "DELETE FROM schema.table1@\"DBLINK.FQDN.COM@USER\"", + "output": "DELETE FROM schema.table1@\"DBLINK.FQDN.COM@USER\"" + }, { "input": "CALL", "output": "CALL" }, { "input": "MERGE INTO TEST USING (SELECT * FROM DUAL) SOURCE on (TEST.X=SOURCE.DUMMY) WHEN NOT MATCHED THEN INSERT VALUES(SOURCE.DUMMY)", - "output": "MERGE TEST" + "output": "MERGE INTO TEST" }, { "input": "MERGE INTO TEST ALIAS USING (SELECT * FROM DUAL) SOURCE on (TEST.X=SOURCE.DUMMY) WHEN NOT MATCHED THEN INSERT VALUES(SOURCE.DUMMY)", - "output": "MERGE TEST" + "output": "MERGE INTO TEST" }, { "input": "MERGE INTO SCHEMA.TEST USING (SELECT * FROM DUAL) SOURCE on (TEST.X=SOURCE.DUMMY) WHEN NOT MATCHED THEN INSERT VALUES(SOURCE.DUMMY)", - "output": "MERGE SCHEMA.TEST" + "output": "MERGE INTO SCHEMA.TEST" }, { "input": "MERGE INTO SCHEMA.TEST@DBLINK USING (SELECT * FROM DUAL) SOURCE on (TEST.X=SOURCE.DUMMY) WHEN NOT MATCHED THEN INSERT VALUES(SOURCE.DUMMY)", - "output": "MERGE SCHEMA.TEST@DBLINK" + "output": "MERGE INTO SCHEMA.TEST@DBLINK" }, { "input": "MERGE INTO SCHEMA.TEST@\"DBLINK.FQDN.COM\" USING (SELECT * FROM DUAL) SOURCE on (TEST.X=SOURCE.DUMMY) WHEN NOT MATCHED THEN INSERT VALUES(SOURCE.DUMMY)", - "output": "MERGE SCHEMA.TEST@\"DBLINK.FQDN.COM\"" + "output": "MERGE INTO SCHEMA.TEST@\"DBLINK.FQDN.COM\"" + }, + { + "input": "MERGE INTO SCHEMA.TEST@\"DBLINK.FQDN.COM@USER\" USING (SELECT * FROM DUAL) SOURCE on (TEST.X=SOURCE.DUMMY) WHEN NOT MATCHED THEN INSERT VALUES(SOURCE.DUMMY)", + "output": "MERGE INTO SCHEMA.TEST@\"DBLINK.FQDN.COM@USER\"" + }, + { + "input": "MERGE", + "output": "MERGE" }, { "input": "{ ? = call FUNCTION() }", "output": "CALL FUNCTION" }, + { + "input": "{ ? = call SCHEMA.FUNCTION() }", + "output": "CALL SCHEMA.FUNCTION" + }, + { + "input": "{ ? = call SCHEMA.FUNCTION@\"DBLINK.FQDN.COM@USER\"() }", + "output": "CALL SCHEMA.FUNCTION@\"DBLINK.FQDN.COM@USER\"" + }, { "input": "{ call PROCEDURE() }", "output": "CALL PROCEDURE" }, + { + "input": "{call SCHEMA.PROCEDURE() }", + "output": "CALL SCHEMA.PROCEDURE" + }, + { + "input": "{call SCHEMA.PROCEDURE@\"DBLINK.FQDN.COM@USER\"() }", + "output": "CALL SCHEMA.PROCEDURE@\"DBLINK.FQDN.COM@USER\"" + }, { "input": "{call PROCEDURE() }", "output": "CALL PROCEDURE" From ac11700b560018a0f16406a988597042b915f0d5 Mon Sep 17 00:00:00 2001 From: wolframhaussig <13997737+wolframhaussig@users.noreply.github.com> Date: Mon, 1 Jul 2019 14:17:56 +0200 Subject: [PATCH 03/14] further SQL parsing improvement (#676) remove db link from Span name TODO: define field where db link should be stored --- .../agent/jdbc/signature/SignatureParser.java | 40 ++++++++++++------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java index c786adca1d..9f08148d5e 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java @@ -58,9 +58,10 @@ public class SignatureParser { private final static ConcurrentMap signatureCache = new ConcurrentHashMap(DISABLE_CACHE_THRESHOLD, 0.5f, Runtime.getRuntime().availableProcessors()); private final Scanner scanner = new Scanner(); + private final StringBuilder dbLink = new StringBuilder(); public void querySignature(String query, StringBuilder signature, boolean preparedStatement) { - + dbLink.setLength(0); final boolean cacheable = preparedStatement // non-prepared statements are likely to be dynamic strings && QUERY_LENGTH_CACHE_LOWER_THRESHOLD < query.length() && query.length() < QUERY_LENGTH_CACHE_UPPER_THRESHOLD; @@ -127,7 +128,8 @@ private void parse(String query, StringBuilder signature) { case UPDATE: signature.append("UPDATE"); // Scan for the table name - boolean hasPeriod = false, hasFirstPeriod = false, inQuotes = false; + boolean hasPeriod = false, hasFirstPeriod = false, inQuotes = false, inDbLink = false; + StringBuilder targetBuilder=signature; if (scanner.scanToken(IDENT)) { signature.append(' '); scanner.appendCurrentTokenText(signature); @@ -135,18 +137,18 @@ private void parse(String query, StringBuilder signature) { switch (t) { case IDENT: if (hasPeriod) { - scanner.appendCurrentTokenText(signature); + scanner.appendCurrentTokenText(targetBuilder); hasPeriod = false; } else if(inQuotes) { - scanner.appendCurrentTokenText(signature); + scanner.appendCurrentTokenText(targetBuilder); } if (!hasFirstPeriod && !inQuotes) { // Some dialects allow option keywords before the table name // example: UPDATE IGNORE foo.bar - signature.setLength(0); - signature.append("UPDATE "); - scanner.appendCurrentTokenText(signature); + targetBuilder.setLength(0); + targetBuilder.append("UPDATE "); + scanner.appendCurrentTokenText(targetBuilder); } // Two adjacent identifiers found after the first period. // Ignore the secondary ones, in case they are unknown keywords. @@ -154,15 +156,19 @@ else if(inQuotes) { case PERIOD: hasFirstPeriod = true; hasPeriod = true; - signature.append('.'); + targetBuilder.append('.'); break; case AT: hasFirstPeriod = true; hasPeriod = true; - signature.append('@'); + if(inDbLink) { + targetBuilder.append('@'); + } else { + targetBuilder = dbLink; + inDbLink = true; + } break; case DQUOT: - signature.append('"'); inQuotes = !inQuotes; break; default: @@ -190,27 +196,33 @@ private void appendIdentifiers(StringBuilder signature) { scanner.appendCurrentTokenText(signature); boolean connectedIdents = false; boolean inQuotes = false; + boolean inDbLink = false; + StringBuilder targetBuilder=signature; for (Scanner.Token t = scanner.scan(false); t != EOF; t = scanner.scan(false)) { switch (t) { case IDENT: // do not add tokens which are separated by a space if (connectedIdents) { - scanner.appendCurrentTokenText(signature); + scanner.appendCurrentTokenText(targetBuilder); connectedIdents = false; } else { return; } break; case PERIOD: - signature.append('.'); + targetBuilder.append('.'); connectedIdents = true; break; case AT: - signature.append('@'); connectedIdents = true; + if(inDbLink) { + targetBuilder.append('@'); + } else { + targetBuilder = dbLink; + inDbLink = true; + } break; case DQUOT: - signature.append('"'); inQuotes = !inQuotes; break; case USING: From a8b4266f6f3844c9634af82d5a865ab8fb82efc4 Mon Sep 17 00:00:00 2001 From: wolframhaussig <13997737+wolframhaussig@users.noreply.github.com> Date: Mon, 1 Jul 2019 15:48:12 +0200 Subject: [PATCH 04/14] further SQL parsing improvement (#676) remove JDBC escape logic from Scanner --- .../apm/agent/jdbc/signature/Scanner.java | 10 +- .../agent/jdbc/signature/SignatureParser.java | 4 +- .../agent/jdbc/signature/filter/Filter.java | 29 ++++++ .../jdbc/signature/filter/FilterChain.java | 51 ++++++++++ .../jdbc/signature/filter/JdbcFilter.java | 96 +++++++++++++++++++ .../src/test/resources/signature_tests.json | 23 +++-- 6 files changed, 194 insertions(+), 19 deletions(-) create mode 100644 apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/filter/Filter.java create mode 100644 apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/filter/FilterChain.java create mode 100644 apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/filter/JdbcFilter.java diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/Scanner.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/Scanner.java index ef978e0394..f8e3e657ab 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/Scanner.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/Scanner.java @@ -68,6 +68,7 @@ public boolean scanToken(Token token) { } return false; } + public Token scan() { return scan(true); } @@ -108,9 +109,6 @@ public Token scan(boolean skipDoubleQuotes) { case '[': // T-SQL bracket-quoted identifier return scanQuotedIdentifier(']'); - case '{': - // JDBC escapes - return scan(skipDoubleQuotes); case '`': // MySQL-style backtick-quoted identifier return scanQuotedIdentifier('`'); @@ -136,12 +134,6 @@ public Token scan(boolean skipDoubleQuotes) { return Token.PERIOD; case '@': return Token.AT; - case '?': - //skip JDBC variables - return scan(skipDoubleQuotes); - case '=': - //skip equals as in ? = call function() - return scan(skipDoubleQuotes); case '$': if (!hasNext()) { return Token.OTHER; diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java index 9f08148d5e..ce8fd9917e 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java @@ -27,6 +27,8 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import co.elastic.apm.agent.jdbc.signature.filter.FilterChain; + import static co.elastic.apm.agent.jdbc.signature.Scanner.Token.EOF; import static co.elastic.apm.agent.jdbc.signature.Scanner.Token.FROM; import static co.elastic.apm.agent.jdbc.signature.Scanner.Token.IDENT; @@ -73,7 +75,7 @@ public void querySignature(String query, StringBuilder signature, boolean prepar } } - scanner.setQuery(query); + scanner.setQuery(FilterChain.DEFAULT_CHAIN.doFilter(query)); parse(query, signature); if (cacheable && signatureCache.size() <= DISABLE_CACHE_THRESHOLD) { diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/filter/Filter.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/filter/Filter.java new file mode 100644 index 0000000000..1a52464d90 --- /dev/null +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/filter/Filter.java @@ -0,0 +1,29 @@ +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 - 2019 Elastic and contributors + * %% + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. 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. + * #L% + */ +package co.elastic.apm.agent.jdbc.signature.filter; + +public interface Filter { + String doFilter(String sql); +} diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/filter/FilterChain.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/filter/FilterChain.java new file mode 100644 index 0000000000..ee89639c82 --- /dev/null +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/filter/FilterChain.java @@ -0,0 +1,51 @@ +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 - 2019 Elastic and contributors + * %% + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. 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. + * #L% + */ +package co.elastic.apm.agent.jdbc.signature.filter; + +import java.util.ArrayList; +import java.util.List; + +public class FilterChain implements Filter { + public static final FilterChain DEFAULT_CHAIN = new FilterChain(new JdbcFilter()); + private final List filter=new ArrayList<>(); + public FilterChain() {} + public FilterChain(Filter... filter) { + for(int i=0;i cache=new ThreadLocal() { + @Override protected StringBuilder initialValue() { + return new StringBuilder(); + } + }; + private int findFirstNonWhitespace(String str, int pos) { + for(int i = pos+1; i < str.length(); i++){ + if(!Character.isWhitespace(str.charAt(i))){ + return i; + } + } + return -1; + } + @Override + public String doFilter(String sql) { + if(!sql.contains("{")) { + return sql; + } + StringBuilder sb = cache.get(); + sb.setLength(0); + boolean inQuote = false, inJdbcEscape = false; + char c; + int i = 0,j = 0; + while(ii) { + i=j; + } + if(sql.toLowerCase().indexOf("oj", i) == i) { + i+=2; + } + break; + case '}': + if(inQuote) { + sb.append(c); + }else { + inJdbcEscape = false; + } + i++; + break; + case '?': + case '=': + if(inQuote || !inJdbcEscape) { + sb.append(c); + } + i++; + break; + case '\'': + sb.append(c); + inQuote = !inQuote; + i++; + break; + default: + sb.append(c); + i++; + break; + } + } + return sb.toString(); + } +} diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/test/resources/signature_tests.json b/apm-agent-plugins/apm-jdbc-plugin/src/test/resources/signature_tests.json index eddd3c5630..43d46139a6 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/test/resources/signature_tests.json +++ b/apm-agent-plugins/apm-jdbc-plugin/src/test/resources/signature_tests.json @@ -51,10 +51,15 @@ "comment": "We capture the first table of the outermost select statement", "input": "SELECT *,(SELECT COUNT(*) FROM table2 WHERE table2.field1 = table1.id) AS count FROM table1 WHERE table1.field1 = 'value'", "output": "SELECT FROM table1" + }, + { + "comment": "JDBC escape syntax join", + "input": "SELECT * FROM {oj Countries JOIN Cities ON (Countries.country_ISO_code=Cities.country_ISO_code)}", + "output": "SELECT FROM Countries" }, { "input": "SELECT *,(SELECT COUNT(*) FROM table2 WHERE table2.field1 = table1.id) AS count FROM schema.table1@\"DBLINK.FQDN.COM@USER\" WHERE table1.field1 = 'value'", - "output": "SELECT FROM schema.table1@\"DBLINK.FQDN.COM@USER\"" + "output": "SELECT FROM schema.table1" }, { "comment": "If the outermost select operates on derived tables, then we just return 'SELECT' (i.e. the fallback)", @@ -71,7 +76,7 @@ }, { "input": "UPDATE IGNORE foo.bar@\"DBLINK.FQDN.COM@USER\" SET bar=1 WHERE baz=2", - "output": "UPDATE foo.bar@\"DBLINK.FQDN.COM@USER\"" + "output": "UPDATE foo.bar" }, { "input": "UPDATE ONLY foo AS bar SET baz=1", @@ -87,7 +92,7 @@ }, { "input": "INSERT LOW_PRIORITY IGNORE INTO schema.table1@\"DBLINK.FQDN.COM@USER\" (col) VALUES(?)", - "output": "INSERT INTO schema.table1@\"DBLINK.FQDN.COM@USER\"" + "output": "INSERT INTO schema.table1" }, { "input": "CALL foo(bar, 123)", @@ -153,7 +158,7 @@ }, { "input": "DELETE FROM schema.table1@\"DBLINK.FQDN.COM@USER\"", - "output": "DELETE FROM schema.table1@\"DBLINK.FQDN.COM@USER\"" + "output": "DELETE FROM schema.table1" }, { "input": "CALL", @@ -173,15 +178,15 @@ }, { "input": "MERGE INTO SCHEMA.TEST@DBLINK USING (SELECT * FROM DUAL) SOURCE on (TEST.X=SOURCE.DUMMY) WHEN NOT MATCHED THEN INSERT VALUES(SOURCE.DUMMY)", - "output": "MERGE INTO SCHEMA.TEST@DBLINK" + "output": "MERGE INTO SCHEMA.TEST" }, { "input": "MERGE INTO SCHEMA.TEST@\"DBLINK.FQDN.COM\" USING (SELECT * FROM DUAL) SOURCE on (TEST.X=SOURCE.DUMMY) WHEN NOT MATCHED THEN INSERT VALUES(SOURCE.DUMMY)", - "output": "MERGE INTO SCHEMA.TEST@\"DBLINK.FQDN.COM\"" + "output": "MERGE INTO SCHEMA.TEST" }, { "input": "MERGE INTO SCHEMA.TEST@\"DBLINK.FQDN.COM@USER\" USING (SELECT * FROM DUAL) SOURCE on (TEST.X=SOURCE.DUMMY) WHEN NOT MATCHED THEN INSERT VALUES(SOURCE.DUMMY)", - "output": "MERGE INTO SCHEMA.TEST@\"DBLINK.FQDN.COM@USER\"" + "output": "MERGE INTO SCHEMA.TEST" }, { "input": "MERGE", @@ -197,7 +202,7 @@ }, { "input": "{ ? = call SCHEMA.FUNCTION@\"DBLINK.FQDN.COM@USER\"() }", - "output": "CALL SCHEMA.FUNCTION@\"DBLINK.FQDN.COM@USER\"" + "output": "CALL SCHEMA.FUNCTION" }, { "input": "{ call PROCEDURE() }", @@ -209,7 +214,7 @@ }, { "input": "{call SCHEMA.PROCEDURE@\"DBLINK.FQDN.COM@USER\"() }", - "output": "CALL SCHEMA.PROCEDURE@\"DBLINK.FQDN.COM@USER\"" + "output": "CALL SCHEMA.PROCEDURE" }, { "input": "{call PROCEDURE() }", From d8f6395f5bc27290c75d41125928a7c1411f64d2 Mon Sep 17 00:00:00 2001 From: wolframhaussig <13997737+wolframhaussig@users.noreply.github.com> Date: Tue, 2 Jul 2019 08:27:25 +0200 Subject: [PATCH 05/14] further SQL parsing improvement (#676) reworked db link logic --- .../apm/agent/jdbc/signature/Scanner.java | 13 +-- .../agent/jdbc/signature/SignatureParser.java | 104 ++++++++---------- .../jdbc/signature/SignatureParserTest.java | 56 +++++++++- 3 files changed, 96 insertions(+), 77 deletions(-) diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/Scanner.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/Scanner.java index f8e3e657ab..4e5c19b43b 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/Scanner.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/Scanner.java @@ -70,9 +70,6 @@ public boolean scanToken(Token token) { } public Token scan() { - return scan(true); - } - public Token scan(boolean skipDoubleQuotes) { if (!hasNext()) { return Token.EOF; } @@ -101,11 +98,7 @@ public Token scan(boolean skipDoubleQuotes) { // string literal delimiter by default, // but we assume standard SQL and treat // it as a identifier delimiter. - if(skipDoubleQuotes) { - return scanQuotedIdentifier('"'); - }else { - return Token.DQUOT; - } + return scanQuotedIdentifier('"'); case '[': // T-SQL bracket-quoted identifier return scanQuotedIdentifier(']'); @@ -132,8 +125,6 @@ public Token scan(boolean skipDoubleQuotes) { return Token.OTHER; case '.': return Token.PERIOD; - case '@': - return Token.AT; case '$': if (!hasNext()) { return Token.OTHER; @@ -369,8 +360,6 @@ public enum Token { PERIOD, // . LPAREN, // ( RPAREN, // ) - AT, // @ - DQUOT, // " AS, CALL, diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java index ce8fd9917e..31350f9dff 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java @@ -57,46 +57,48 @@ public class SignatureParser { * When relying on weak keys, we would not leverage any caching benefits if the query string is collected. * That means that we are leaking Strings but as the size of the map is limited that should not be an issue. */ - private final static ConcurrentMap signatureCache = new ConcurrentHashMap(DISABLE_CACHE_THRESHOLD, 0.5f, Runtime.getRuntime().availableProcessors()); + private final static ConcurrentMap signatureCache = new ConcurrentHashMap(DISABLE_CACHE_THRESHOLD, 0.5f, Runtime.getRuntime().availableProcessors()); private final Scanner scanner = new Scanner(); - private final StringBuilder dbLink = new StringBuilder(); public void querySignature(String query, StringBuilder signature, boolean preparedStatement) { - dbLink.setLength(0); + querySignature(query, signature, new StringBuilder(), preparedStatement); + } + public void querySignature(String query, StringBuilder signature, StringBuilder dbLink, boolean preparedStatement) { final boolean cacheable = preparedStatement // non-prepared statements are likely to be dynamic strings && QUERY_LENGTH_CACHE_LOWER_THRESHOLD < query.length() && query.length() < QUERY_LENGTH_CACHE_UPPER_THRESHOLD; if (cacheable) { - final String cachedSignature = signatureCache.get(query); + final String[] cachedSignature = signatureCache.get(query); if (cachedSignature != null) { - signature.append(cachedSignature); + signature.append(cachedSignature[0]); + dbLink.append(cachedSignature[1]); return; } } scanner.setQuery(FilterChain.DEFAULT_CHAIN.doFilter(query)); - parse(query, signature); + parse(query, signature, dbLink); if (cacheable && signatureCache.size() <= DISABLE_CACHE_THRESHOLD) { // we don't mind a small overshoot due to race conditions - signatureCache.put(query, signature.toString()); + signatureCache.put(query, new String[] {signature.toString(), dbLink.toString()}); } } - private void parse(String query, StringBuilder signature) { + private void parse(String query, StringBuilder signature, StringBuilder dbLink) { final Scanner.Token firstToken = scanner.scanWhile(Scanner.Token.COMMENT); switch (firstToken) { case CALL: signature.append("CALL"); if(scanner.scanUntil(Scanner.Token.IDENT)) { - appendIdentifiers(signature); + appendIdentifiers(signature, dbLink); } return; case DELETE: signature.append("DELETE"); if (scanner.scanUntil(FROM) && scanner.scanUntil(Scanner.Token.IDENT)) { signature.append(" FROM"); - appendIdentifiers(signature); + appendIdentifiers(signature, dbLink); } return; case INSERT: @@ -104,7 +106,7 @@ private void parse(String query, StringBuilder signature) { signature.append(firstToken.name()); if (scanner.scanUntil(Scanner.Token.INTO) && scanner.scanUntil(Scanner.Token.IDENT)) { signature.append(" INTO"); - appendIdentifiers(signature); + appendIdentifiers(signature, dbLink); } return; case SELECT: @@ -119,7 +121,7 @@ private void parse(String query, StringBuilder signature) { if (level == 0) { if (scanner.scanToken(Scanner.Token.IDENT)) { signature.append(" FROM"); - appendIdentifiers(signature); + appendIdentifiers(signature, dbLink); } else { return; } @@ -130,51 +132,43 @@ private void parse(String query, StringBuilder signature) { case UPDATE: signature.append("UPDATE"); // Scan for the table name - boolean hasPeriod = false, hasFirstPeriod = false, inQuotes = false, inDbLink = false; - StringBuilder targetBuilder=signature; + boolean hasPeriod = false, hasFirstPeriod = false, isDbLink = false; if (scanner.scanToken(IDENT)) { signature.append(' '); scanner.appendCurrentTokenText(signature); - for (Scanner.Token t = scanner.scan(false); t != EOF; t = scanner.scan(false)) { + for (Scanner.Token t = scanner.scan(); t != EOF; t = scanner.scan()) { switch (t) { case IDENT: if (hasPeriod) { - scanner.appendCurrentTokenText(targetBuilder); + scanner.appendCurrentTokenText(signature); hasPeriod = false; } - else if(inQuotes) { - scanner.appendCurrentTokenText(targetBuilder); - } - if (!hasFirstPeriod && !inQuotes) { + if (!hasFirstPeriod) { // Some dialects allow option keywords before the table name // example: UPDATE IGNORE foo.bar - targetBuilder.setLength(0); - targetBuilder.append("UPDATE "); - scanner.appendCurrentTokenText(targetBuilder); + signature.setLength(0); + signature.append("UPDATE "); + scanner.appendCurrentTokenText(signature); } + else if(isDbLink) { + scanner.appendCurrentTokenText(dbLink); + isDbLink = false; + } // Two adjacent identifiers found after the first period. // Ignore the secondary ones, in case they are unknown keywords. break; case PERIOD: hasFirstPeriod = true; hasPeriod = true; - targetBuilder.append('.'); + signature.append('.'); break; - case AT: - hasFirstPeriod = true; - hasPeriod = true; - if(inDbLink) { - targetBuilder.append('@'); - } else { - targetBuilder = dbLink; - inDbLink = true; - } - break; - case DQUOT: - inQuotes = !inQuotes; - break; default: - return; + if("@".equals(scanner.text())) { + isDbLink = true; + break; + }else { + return; + } } } } @@ -183,7 +177,7 @@ else if(inQuotes) { signature.append("MERGE"); if(scanner.scanToken(INTO) && scanner.scanUntil(Scanner.Token.IDENT)) { signature.append(" INTO"); - appendIdentifiers(signature); + appendIdentifiers(signature, dbLink); } return; default: @@ -193,43 +187,35 @@ else if(inQuotes) { } } - private void appendIdentifiers(StringBuilder signature) { + private void appendIdentifiers(StringBuilder signature, StringBuilder dbLink) { signature.append(' '); scanner.appendCurrentTokenText(signature); - boolean connectedIdents = false; - boolean inQuotes = false; - boolean inDbLink = false; - StringBuilder targetBuilder=signature; - for (Scanner.Token t = scanner.scan(false); t != EOF; t = scanner.scan(false)) { + boolean connectedIdents = false, isDbLink = false; + for (Scanner.Token t = scanner.scan(); t != EOF; t = scanner.scan()) { switch (t) { case IDENT: // do not add tokens which are separated by a space if (connectedIdents) { - scanner.appendCurrentTokenText(targetBuilder); + scanner.appendCurrentTokenText(signature); connectedIdents = false; } else { + if(isDbLink) { + scanner.appendCurrentTokenText(dbLink); + isDbLink = false; + } return; } break; case PERIOD: - targetBuilder.append('.'); + signature.append('.'); connectedIdents = true; break; - case AT: - connectedIdents = true; - if(inDbLink) { - targetBuilder.append('@'); - } else { - targetBuilder = dbLink; - inDbLink = true; - } - break; - case DQUOT: - inQuotes = !inQuotes; - break; case USING: return; default: + if("@".equals(scanner.text())) { + isDbLink = true; + } break; } } diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/signature/SignatureParserTest.java b/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/signature/SignatureParserTest.java index 961660d2f8..eae9c50e86 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/signature/SignatureParserTest.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/signature/SignatureParserTest.java @@ -26,20 +26,21 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import org.junit.jupiter.api.BeforeEach; + +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; class SignatureParserTest { - private SignatureParser signatureParser; - private JsonNode testCases; + private static SignatureParser signatureParser; + private static JsonNode testCases; - @BeforeEach - void setUp() throws Exception { + @BeforeAll + static void setUp() throws Exception { signatureParser = new SignatureParser(); - testCases = new ObjectMapper().readTree(getClass().getResource("/signature_tests.json")); + testCases = new ObjectMapper().readTree(SignatureParserTest.class.getResource("/signature_tests.json")); } @Test @@ -49,6 +50,49 @@ void testScanner() { assertThat(getSignature(testCase.get("input").asText())).isEqualTo(testCase.get("output").asText()); } } + + @Test + void testDbLinkForUpdate() { + final StringBuilder sb = new StringBuilder(); + final StringBuilder dblink = new StringBuilder(); + signatureParser.querySignature("UPDATE foo.bar@\"DBLINK.FQDN.COM@USER\" SET bar=1 WHERE baz=2", sb, dblink, false); + assertThat(dblink.toString()).isEqualTo("DBLINK.FQDN.COM@USER"); + } + + @Test + void testDbLink() { + final StringBuilder sb = new StringBuilder(); + final StringBuilder dblink = new StringBuilder(); + signatureParser.querySignature("SELECT * FROM TABLE1@DBLINK", sb, dblink, false); + assertThat(dblink.toString()).isEqualTo("DBLINK"); + } + + @Test + void testDbLinkCache() { + final StringBuilder sb = new StringBuilder(); + final StringBuilder dblink = new StringBuilder(); + signatureParser.querySignature("SELECT * FROM TABLE1@DBLINK", sb, dblink, true); + sb.setLength(0); + dblink.setLength(0); + signatureParser.querySignature("SELECT * FROM TABLE1@DBLINK", sb, dblink, true); + assertThat(dblink.toString()).isEqualTo("DBLINK"); + } + + @Test + void testDbLinkFqdn() { + final StringBuilder sb = new StringBuilder(); + final StringBuilder dblink = new StringBuilder(); + signatureParser.querySignature("SELECT * FROM TABLE1@\"DBLINK.FQDN.COM\"", sb, dblink, false); + assertThat(dblink.toString()).isEqualTo("DBLINK.FQDN.COM"); + } + + @Test + void testDbLinkFqdnWithUser() { + final StringBuilder sb = new StringBuilder(); + final StringBuilder dblink = new StringBuilder(); + signatureParser.querySignature("SELECT * FROM TABLE1@\"DBLINK.FQDN.COM@USER\"", sb, dblink, false); + assertThat(dblink.toString()).isEqualTo("DBLINK.FQDN.COM@USER"); + } String getSignature(String query) { final StringBuilder sb = new StringBuilder(); From 6a0a0b4d1757b9801133f295d7357a10cd6ff08d Mon Sep 17 00:00:00 2001 From: wolframhaussig <13997737+wolframhaussig@users.noreply.github.com> Date: Fri, 12 Jul 2019 15:37:46 +0200 Subject: [PATCH 06/14] Update apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java Co-Authored-By: Felix Barnsteiner --- .../co/elastic/apm/agent/jdbc/signature/SignatureParser.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java index 31350f9dff..78869d9b02 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java @@ -90,7 +90,7 @@ private void parse(String query, StringBuilder signature, StringBuilder dbLink) switch (firstToken) { case CALL: signature.append("CALL"); - if(scanner.scanUntil(Scanner.Token.IDENT)) { + if (scanner.scanUntil(Scanner.Token.IDENT)) { appendIdentifiers(signature, dbLink); } return; From 14486fbad8c4a31cc9160f18e3d61cf2f73985eb Mon Sep 17 00:00:00 2001 From: wolframhaussig <13997737+wolframhaussig@users.noreply.github.com> Date: Mon, 15 Jul 2019 08:19:05 +0200 Subject: [PATCH 07/14] further SQL parsing improvement (elastic#676) make dbLink nullable minor formatting fix --- .../agent/jdbc/signature/SignatureParser.java | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java index 78869d9b02..65120a6399 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java @@ -27,6 +27,8 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import javax.annotation.Nullable; + import co.elastic.apm.agent.jdbc.signature.filter.FilterChain; import static co.elastic.apm.agent.jdbc.signature.Scanner.Token.EOF; @@ -62,9 +64,10 @@ public class SignatureParser { private final Scanner scanner = new Scanner(); public void querySignature(String query, StringBuilder signature, boolean preparedStatement) { - querySignature(query, signature, new StringBuilder(), preparedStatement); + querySignature(query, signature, null, preparedStatement); } - public void querySignature(String query, StringBuilder signature, StringBuilder dbLink, boolean preparedStatement) { + + public void querySignature(String query, StringBuilder signature, @Nullable StringBuilder dbLink, boolean preparedStatement) { final boolean cacheable = preparedStatement // non-prepared statements are likely to be dynamic strings && QUERY_LENGTH_CACHE_LOWER_THRESHOLD < query.length() && query.length() < QUERY_LENGTH_CACHE_UPPER_THRESHOLD; @@ -72,7 +75,9 @@ public void querySignature(String query, StringBuilder signature, StringBuilder final String[] cachedSignature = signatureCache.get(query); if (cachedSignature != null) { signature.append(cachedSignature[0]); - dbLink.append(cachedSignature[1]); + if(dbLink != null) { + dbLink.append(cachedSignature[1]); + } return; } } @@ -82,10 +87,11 @@ public void querySignature(String query, StringBuilder signature, StringBuilder if (cacheable && signatureCache.size() <= DISABLE_CACHE_THRESHOLD) { // we don't mind a small overshoot due to race conditions - signatureCache.put(query, new String[] {signature.toString(), dbLink.toString()}); + signatureCache.put(query, new String[] {signature.toString(), dbLink != null ? dbLink.toString() : ""}); } } - private void parse(String query, StringBuilder signature, StringBuilder dbLink) { + + private void parse(String query, StringBuilder signature, @Nullable StringBuilder dbLink) { final Scanner.Token firstToken = scanner.scanWhile(Scanner.Token.COMMENT); switch (firstToken) { case CALL: @@ -151,7 +157,9 @@ private void parse(String query, StringBuilder signature, StringBuilder dbLink) scanner.appendCurrentTokenText(signature); } else if(isDbLink) { - scanner.appendCurrentTokenText(dbLink); + if(dbLink != null) { + scanner.appendCurrentTokenText(dbLink); + } isDbLink = false; } // Two adjacent identifiers found after the first period. @@ -187,7 +195,7 @@ else if(isDbLink) { } } - private void appendIdentifiers(StringBuilder signature, StringBuilder dbLink) { + private void appendIdentifiers(StringBuilder signature, @Nullable StringBuilder dbLink) { signature.append(' '); scanner.appendCurrentTokenText(signature); boolean connectedIdents = false, isDbLink = false; @@ -200,7 +208,9 @@ private void appendIdentifiers(StringBuilder signature, StringBuilder dbLink) { connectedIdents = false; } else { if(isDbLink) { - scanner.appendCurrentTokenText(dbLink); + if(dbLink != null) { + scanner.appendCurrentTokenText(dbLink); + } isDbLink = false; } return; From e323eb2d4e8015786e09c71faeda4016b6521b8f Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 16 Jul 2019 18:00:40 +0200 Subject: [PATCH 08/14] Simplify JDBC escape filtering and eliminate allocations --- .../apm/agent/jdbc/signature/JdbcFilter.java | 70 ++++++++++++++ .../apm/agent/jdbc/signature/Scanner.java | 10 +- .../agent/jdbc/signature/SignatureParser.java | 3 +- .../agent/jdbc/signature/filter/Filter.java | 29 ------ .../jdbc/signature/filter/FilterChain.java | 51 ---------- .../jdbc/signature/filter/JdbcFilter.java | 96 ------------------- 6 files changed, 79 insertions(+), 180 deletions(-) create mode 100644 apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/JdbcFilter.java delete mode 100644 apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/filter/Filter.java delete mode 100644 apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/filter/FilterChain.java delete mode 100644 apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/filter/JdbcFilter.java diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/JdbcFilter.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/JdbcFilter.java new file mode 100644 index 0000000000..396f469d79 --- /dev/null +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/JdbcFilter.java @@ -0,0 +1,70 @@ +/*- + * #%L + * Elastic APM Java agent + * %% + * Copyright (C) 2018 - 2019 Elastic and contributors + * %% + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. 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. + * #L% + */ +package co.elastic.apm.agent.jdbc.signature; + +class JdbcFilter { + + private boolean inQuote = false; + private boolean inJdbcEscape = false; + + boolean skip(Scanner s, char c) { + switch (c) { + case '{': + if (!inQuote) { + inJdbcEscape = true; + return true; + } + break; + case 'o': + case 'O': + if (!inQuote && inJdbcEscape && s.isNextCharIgnoreCase('j')) { + s.next(); + return true; + } + break; + case '}': + if (!inQuote) { + inJdbcEscape = false; + return true; + } + break; + case '?': + case '=': + if (!inQuote && inJdbcEscape) { + return true; + } + break; + case '\'': + inQuote = !inQuote; + break; + } + return false; + } + + void reset() { + inQuote = false; + inJdbcEscape = false; + } +} diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/Scanner.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/Scanner.java index 4e5c19b43b..66873bf703 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/Scanner.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/Scanner.java @@ -31,9 +31,11 @@ public class Scanner { private int end; // text end char offset private int pos; // read position char offset private int inputLength; + private JdbcFilter filter = new JdbcFilter(); public void setQuery(String sql) { this.input = sql; + filter.reset(); inputLength = sql.length(); start = 0; end = 0; @@ -74,7 +76,7 @@ public Token scan() { return Token.EOF; } char c = next(); - while (Character.isSpaceChar(c)) { + while (Character.isSpaceChar(c) || filter.skip(this, c)) { if (hasNext()) { c = next(); } else { @@ -301,7 +303,7 @@ private char peek() { return input.charAt(pos); } - private char next() { + char next() { final char c = peek(); pos++; end = pos; @@ -347,6 +349,10 @@ private boolean isNextChar(char c) { return hasNext() && peek() == c; } + boolean isNextCharIgnoreCase(char c) { + return hasNext() && Character.toLowerCase(peek()) == Character.toLowerCase(c); + } + public enum Token { OTHER, diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java index 65120a6399..030052b689 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java @@ -29,7 +29,6 @@ import javax.annotation.Nullable; -import co.elastic.apm.agent.jdbc.signature.filter.FilterChain; import static co.elastic.apm.agent.jdbc.signature.Scanner.Token.EOF; import static co.elastic.apm.agent.jdbc.signature.Scanner.Token.FROM; @@ -82,7 +81,7 @@ public void querySignature(String query, StringBuilder signature, @Nullable Stri } } - scanner.setQuery(FilterChain.DEFAULT_CHAIN.doFilter(query)); + scanner.setQuery(query); parse(query, signature, dbLink); if (cacheable && signatureCache.size() <= DISABLE_CACHE_THRESHOLD) { diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/filter/Filter.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/filter/Filter.java deleted file mode 100644 index 1a52464d90..0000000000 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/filter/Filter.java +++ /dev/null @@ -1,29 +0,0 @@ -/*- - * #%L - * Elastic APM Java agent - * %% - * Copyright (C) 2018 - 2019 Elastic and contributors - * %% - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. 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. - * #L% - */ -package co.elastic.apm.agent.jdbc.signature.filter; - -public interface Filter { - String doFilter(String sql); -} diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/filter/FilterChain.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/filter/FilterChain.java deleted file mode 100644 index ee89639c82..0000000000 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/filter/FilterChain.java +++ /dev/null @@ -1,51 +0,0 @@ -/*- - * #%L - * Elastic APM Java agent - * %% - * Copyright (C) 2018 - 2019 Elastic and contributors - * %% - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. 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. - * #L% - */ -package co.elastic.apm.agent.jdbc.signature.filter; - -import java.util.ArrayList; -import java.util.List; - -public class FilterChain implements Filter { - public static final FilterChain DEFAULT_CHAIN = new FilterChain(new JdbcFilter()); - private final List filter=new ArrayList<>(); - public FilterChain() {} - public FilterChain(Filter... filter) { - for(int i=0;i cache=new ThreadLocal() { - @Override protected StringBuilder initialValue() { - return new StringBuilder(); - } - }; - private int findFirstNonWhitespace(String str, int pos) { - for(int i = pos+1; i < str.length(); i++){ - if(!Character.isWhitespace(str.charAt(i))){ - return i; - } - } - return -1; - } - @Override - public String doFilter(String sql) { - if(!sql.contains("{")) { - return sql; - } - StringBuilder sb = cache.get(); - sb.setLength(0); - boolean inQuote = false, inJdbcEscape = false; - char c; - int i = 0,j = 0; - while(ii) { - i=j; - } - if(sql.toLowerCase().indexOf("oj", i) == i) { - i+=2; - } - break; - case '}': - if(inQuote) { - sb.append(c); - }else { - inJdbcEscape = false; - } - i++; - break; - case '?': - case '=': - if(inQuote || !inJdbcEscape) { - sb.append(c); - } - i++; - break; - case '\'': - sb.append(c); - inQuote = !inQuote; - i++; - break; - default: - sb.append(c); - i++; - break; - } - } - return sb.toString(); - } -} From 705c49cd7e6ea85b7889b60964d4b9f95c74ad4b Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 16 Jul 2019 18:18:21 +0200 Subject: [PATCH 09/14] Make filter final --- .../main/java/co/elastic/apm/agent/jdbc/signature/Scanner.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/Scanner.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/Scanner.java index 66873bf703..5d5b6316ed 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/Scanner.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/Scanner.java @@ -31,7 +31,7 @@ public class Scanner { private int end; // text end char offset private int pos; // read position char offset private int inputLength; - private JdbcFilter filter = new JdbcFilter(); + private final JdbcFilter filter = new JdbcFilter(); public void setQuery(String sql) { this.input = sql; From 183bc0c797364d3fe2eb5b28f256ae3f8963dd58 Mon Sep 17 00:00:00 2001 From: wolframhaussig <13997737+wolframhaussig@users.noreply.github.com> Date: Wed, 17 Jul 2019 07:21:17 +0200 Subject: [PATCH 10/14] added edge case testcase --- .../apm-jdbc-plugin/src/test/resources/signature_tests.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/test/resources/signature_tests.json b/apm-agent-plugins/apm-jdbc-plugin/src/test/resources/signature_tests.json index 43d46139a6..1ac1ec59cb 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/test/resources/signature_tests.json +++ b/apm-agent-plugins/apm-jdbc-plugin/src/test/resources/signature_tests.json @@ -196,6 +196,10 @@ "input": "{ ? = call FUNCTION() }", "output": "CALL FUNCTION" }, + { + "input": "{ ? = call oj() }", + "output": "CALL oj" + }, { "input": "{ ? = call SCHEMA.FUNCTION() }", "output": "CALL SCHEMA.FUNCTION" From 19b0a3bb66126c142afa377ca0bad1bc418ef86b Mon Sep 17 00:00:00 2001 From: wolframhaussig <13997737+wolframhaussig@users.noreply.github.com> Date: Wed, 17 Jul 2019 07:27:04 +0200 Subject: [PATCH 11/14] fix edge case when detecting procedure name using jdbc escape syntax --- .../co/elastic/apm/agent/jdbc/signature/JdbcFilter.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/JdbcFilter.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/JdbcFilter.java index 396f469d79..98d26af4eb 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/JdbcFilter.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/JdbcFilter.java @@ -28,19 +28,22 @@ class JdbcFilter { private boolean inQuote = false; private boolean inJdbcEscape = false; + private boolean jdbcKeyWord = false; boolean skip(Scanner s, char c) { switch (c) { case '{': if (!inQuote) { inJdbcEscape = true; + jdbcKeyWord = true; return true; } break; case 'o': case 'O': - if (!inQuote && inJdbcEscape && s.isNextCharIgnoreCase('j')) { + if (!inQuote && inJdbcEscape && jdbcKeyWord && s.isNextCharIgnoreCase('j')) { s.next(); + jdbcKeyWord = false; return true; } break; @@ -60,11 +63,13 @@ boolean skip(Scanner s, char c) { inQuote = !inQuote; break; } + jdbcKeyWord = false; return false; } void reset() { inQuote = false; inJdbcEscape = false; + jdbcKeyWord = false; } } From 15cacb95817a74e907fc508cf637402e74208e03 Mon Sep 17 00:00:00 2001 From: wolframhaussig <13997737+wolframhaussig@users.noreply.github.com> Date: Wed, 17 Jul 2019 11:00:08 +0200 Subject: [PATCH 12/14] formatting --- .../apm/agent/jdbc/signature/JdbcFilter.java | 60 +++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/JdbcFilter.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/JdbcFilter.java index 98d26af4eb..edfd5edbc5 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/JdbcFilter.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/JdbcFilter.java @@ -32,36 +32,36 @@ class JdbcFilter { boolean skip(Scanner s, char c) { switch (c) { - case '{': - if (!inQuote) { - inJdbcEscape = true; - jdbcKeyWord = true; - return true; - } - break; - case 'o': - case 'O': - if (!inQuote && inJdbcEscape && jdbcKeyWord && s.isNextCharIgnoreCase('j')) { - s.next(); - jdbcKeyWord = false; - return true; - } - break; - case '}': - if (!inQuote) { - inJdbcEscape = false; - return true; - } - break; - case '?': - case '=': - if (!inQuote && inJdbcEscape) { - return true; - } - break; - case '\'': - inQuote = !inQuote; - break; + case '{': + if (!inQuote) { + inJdbcEscape = true; + jdbcKeyWord = true; + return true; + } + break; + case 'o': + case 'O': + if (!inQuote && inJdbcEscape && jdbcKeyWord && s.isNextCharIgnoreCase('j')) { + s.next(); + jdbcKeyWord = false; + return true; + } + break; + case '}': + if (!inQuote) { + inJdbcEscape = false; + return true; + } + break; + case '?': + case '=': + if (!inQuote && inJdbcEscape) { + return true; + } + break; + case '\'': + inQuote = !inQuote; + break; } jdbcKeyWord = false; return false; From 0ce009bd240de6626b83a95c2a0fcbb2492a1c74 Mon Sep 17 00:00:00 2001 From: wolframhaussig <13997737+wolframhaussig@users.noreply.github.com> Date: Wed, 17 Jul 2019 13:11:15 +0200 Subject: [PATCH 13/14] formatting --- .../agent/jdbc/signature/SignatureParser.java | 49 ++++++++++--------- .../jdbc/signature/SignatureParserTest.java | 8 +-- 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java index 030052b689..7c39c730e4 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java @@ -58,12 +58,13 @@ public class SignatureParser { * When relying on weak keys, we would not leverage any caching benefits if the query string is collected. * That means that we are leaking Strings but as the size of the map is limited that should not be an issue. */ - private final static ConcurrentMap signatureCache = new ConcurrentHashMap(DISABLE_CACHE_THRESHOLD, 0.5f, Runtime.getRuntime().availableProcessors()); + private final static ConcurrentMap signatureCache = new ConcurrentHashMap(DISABLE_CACHE_THRESHOLD, + 0.5f, Runtime.getRuntime().availableProcessors()); private final Scanner scanner = new Scanner(); public void querySignature(String query, StringBuilder signature, boolean preparedStatement) { - querySignature(query, signature, null, preparedStatement); + querySignature(query, signature, null, preparedStatement); } public void querySignature(String query, StringBuilder signature, @Nullable StringBuilder dbLink, boolean preparedStatement) { @@ -74,8 +75,8 @@ public void querySignature(String query, StringBuilder signature, @Nullable Stri final String[] cachedSignature = signatureCache.get(query); if (cachedSignature != null) { signature.append(cachedSignature[0]); - if(dbLink != null) { - dbLink.append(cachedSignature[1]); + if (dbLink != null) { + dbLink.append(cachedSignature[1]); } return; } @@ -86,7 +87,7 @@ public void querySignature(String query, StringBuilder signature, @Nullable Stri if (cacheable && signatureCache.size() <= DISABLE_CACHE_THRESHOLD) { // we don't mind a small overshoot due to race conditions - signatureCache.put(query, new String[] {signature.toString(), dbLink != null ? dbLink.toString() : ""}); + signatureCache.put(query, new String[] { signature.toString(), dbLink != null ? dbLink.toString() : "" }); } } @@ -182,7 +183,7 @@ else if(isDbLink) { return; case MERGE: signature.append("MERGE"); - if(scanner.scanToken(INTO) && scanner.scanUntil(Scanner.Token.IDENT)) { + if (scanner.scanToken(INTO) && scanner.scanUntil(Scanner.Token.IDENT)) { signature.append(" INTO"); appendIdentifiers(signature, dbLink); } @@ -202,29 +203,29 @@ private void appendIdentifiers(StringBuilder signature, @Nullable StringBuilder switch (t) { case IDENT: // do not add tokens which are separated by a space - if (connectedIdents) { - scanner.appendCurrentTokenText(signature); - connectedIdents = false; - } else { - if(isDbLink) { - if(dbLink != null) { - scanner.appendCurrentTokenText(dbLink); - } - isDbLink = false; - } - return; - } + if (connectedIdents) { + scanner.appendCurrentTokenText(signature); + connectedIdents = false; + } else { + if (isDbLink) { + if (dbLink != null) { + scanner.appendCurrentTokenText(dbLink); + } + isDbLink = false; + } + return; + } break; case PERIOD: signature.append('.'); - connectedIdents = true; + connectedIdents = true; break; - case USING: - return; + case USING: + return; default: - if("@".equals(scanner.text())) { - isDbLink = true; - } + if ("@".equals(scanner.text())) { + isDbLink = true; + } break; } } diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/signature/SignatureParserTest.java b/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/signature/SignatureParserTest.java index eae9c50e86..61c013a215 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/signature/SignatureParserTest.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/signature/SignatureParserTest.java @@ -58,7 +58,7 @@ void testDbLinkForUpdate() { signatureParser.querySignature("UPDATE foo.bar@\"DBLINK.FQDN.COM@USER\" SET bar=1 WHERE baz=2", sb, dblink, false); assertThat(dblink.toString()).isEqualTo("DBLINK.FQDN.COM@USER"); } - + @Test void testDbLink() { final StringBuilder sb = new StringBuilder(); @@ -66,7 +66,7 @@ void testDbLink() { signatureParser.querySignature("SELECT * FROM TABLE1@DBLINK", sb, dblink, false); assertThat(dblink.toString()).isEqualTo("DBLINK"); } - + @Test void testDbLinkCache() { final StringBuilder sb = new StringBuilder(); @@ -77,7 +77,7 @@ void testDbLinkCache() { signatureParser.querySignature("SELECT * FROM TABLE1@DBLINK", sb, dblink, true); assertThat(dblink.toString()).isEqualTo("DBLINK"); } - + @Test void testDbLinkFqdn() { final StringBuilder sb = new StringBuilder(); @@ -85,7 +85,7 @@ void testDbLinkFqdn() { signatureParser.querySignature("SELECT * FROM TABLE1@\"DBLINK.FQDN.COM\"", sb, dblink, false); assertThat(dblink.toString()).isEqualTo("DBLINK.FQDN.COM"); } - + @Test void testDbLinkFqdnWithUser() { final StringBuilder sb = new StringBuilder(); From 20515ba4dbaa5ea3d954ab0b325c262c5cff0e2d Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Wed, 17 Jul 2019 13:29:39 +0200 Subject: [PATCH 14/14] formatting --- .../apm/agent/jdbc/signature/JdbcFilter.java | 60 ++++++------ .../agent/jdbc/signature/SignatureParser.java | 98 +++++++++---------- .../jdbc/signature/SignatureParserTest.java | 6 +- 3 files changed, 80 insertions(+), 84 deletions(-) diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/JdbcFilter.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/JdbcFilter.java index edfd5edbc5..98d26af4eb 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/JdbcFilter.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/JdbcFilter.java @@ -32,36 +32,36 @@ class JdbcFilter { boolean skip(Scanner s, char c) { switch (c) { - case '{': - if (!inQuote) { - inJdbcEscape = true; - jdbcKeyWord = true; - return true; - } - break; - case 'o': - case 'O': - if (!inQuote && inJdbcEscape && jdbcKeyWord && s.isNextCharIgnoreCase('j')) { - s.next(); - jdbcKeyWord = false; - return true; - } - break; - case '}': - if (!inQuote) { - inJdbcEscape = false; - return true; - } - break; - case '?': - case '=': - if (!inQuote && inJdbcEscape) { - return true; - } - break; - case '\'': - inQuote = !inQuote; - break; + case '{': + if (!inQuote) { + inJdbcEscape = true; + jdbcKeyWord = true; + return true; + } + break; + case 'o': + case 'O': + if (!inQuote && inJdbcEscape && jdbcKeyWord && s.isNextCharIgnoreCase('j')) { + s.next(); + jdbcKeyWord = false; + return true; + } + break; + case '}': + if (!inQuote) { + inJdbcEscape = false; + return true; + } + break; + case '?': + case '=': + if (!inQuote && inJdbcEscape) { + return true; + } + break; + case '\'': + inQuote = !inQuote; + break; } jdbcKeyWord = false; return false; diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java index 7c39c730e4..3234e9cb6e 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/main/java/co/elastic/apm/agent/jdbc/signature/SignatureParser.java @@ -24,18 +24,16 @@ */ package co.elastic.apm.agent.jdbc.signature; +import javax.annotation.Nullable; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import javax.annotation.Nullable; - - import static co.elastic.apm.agent.jdbc.signature.Scanner.Token.EOF; import static co.elastic.apm.agent.jdbc.signature.Scanner.Token.FROM; import static co.elastic.apm.agent.jdbc.signature.Scanner.Token.IDENT; +import static co.elastic.apm.agent.jdbc.signature.Scanner.Token.INTO; import static co.elastic.apm.agent.jdbc.signature.Scanner.Token.LPAREN; import static co.elastic.apm.agent.jdbc.signature.Scanner.Token.RPAREN; -import static co.elastic.apm.agent.jdbc.signature.Scanner.Token.INTO; public class SignatureParser { @@ -66,7 +64,7 @@ public class SignatureParser { public void querySignature(String query, StringBuilder signature, boolean preparedStatement) { querySignature(query, signature, null, preparedStatement); } - + public void querySignature(String query, StringBuilder signature, @Nullable StringBuilder dbLink, boolean preparedStatement) { final boolean cacheable = preparedStatement // non-prepared statements are likely to be dynamic strings && QUERY_LENGTH_CACHE_LOWER_THRESHOLD < query.length() @@ -87,17 +85,17 @@ public void querySignature(String query, StringBuilder signature, @Nullable Stri if (cacheable && signatureCache.size() <= DISABLE_CACHE_THRESHOLD) { // we don't mind a small overshoot due to race conditions - signatureCache.put(query, new String[] { signature.toString(), dbLink != null ? dbLink.toString() : "" }); + signatureCache.put(query, new String[]{signature.toString(), dbLink != null ? dbLink.toString() : ""}); } } - + private void parse(String query, StringBuilder signature, @Nullable StringBuilder dbLink) { final Scanner.Token firstToken = scanner.scanWhile(Scanner.Token.COMMENT); switch (firstToken) { case CALL: signature.append("CALL"); if (scanner.scanUntil(Scanner.Token.IDENT)) { - appendIdentifiers(signature, dbLink); + appendIdentifiers(signature, dbLink); } return; case DELETE: @@ -152,16 +150,15 @@ private void parse(String query, StringBuilder signature, @Nullable StringBuilde if (!hasFirstPeriod) { // Some dialects allow option keywords before the table name // example: UPDATE IGNORE foo.bar - signature.setLength(0); - signature.append("UPDATE "); + signature.setLength(0); + signature.append("UPDATE "); scanner.appendCurrentTokenText(signature); + } else if (isDbLink) { + if (dbLink != null) { + scanner.appendCurrentTokenText(dbLink); + } + isDbLink = false; } - else if(isDbLink) { - if(dbLink != null) { - scanner.appendCurrentTokenText(dbLink); - } - isDbLink = false; - } // Two adjacent identifiers found after the first period. // Ignore the secondary ones, in case they are unknown keywords. break; @@ -171,12 +168,12 @@ else if(isDbLink) { signature.append('.'); break; default: - if("@".equals(scanner.text())) { - isDbLink = true; - break; - }else { + if ("@".equals(scanner.text())) { + isDbLink = true; + break; + } else { return; - } + } } } } @@ -196,38 +193,37 @@ else if(isDbLink) { } private void appendIdentifiers(StringBuilder signature, @Nullable StringBuilder dbLink) { - signature.append(' '); - scanner.appendCurrentTokenText(signature); - boolean connectedIdents = false, isDbLink = false; - for (Scanner.Token t = scanner.scan(); t != EOF; t = scanner.scan()) { - switch (t) { - case IDENT: - // do not add tokens which are separated by a space - if (connectedIdents) { - scanner.appendCurrentTokenText(signature); - connectedIdents = false; - } else { - if (isDbLink) { - if (dbLink != null) { - scanner.appendCurrentTokenText(dbLink); + signature.append(' '); + scanner.appendCurrentTokenText(signature); + boolean connectedIdents = false, isDbLink = false; + for (Scanner.Token t = scanner.scan(); t != EOF; t = scanner.scan()) { + switch (t) { + case IDENT: + // do not add tokens which are separated by a space + if (connectedIdents) { + scanner.appendCurrentTokenText(signature); + connectedIdents = false; + } else { + if (isDbLink) { + if (dbLink != null) { + scanner.appendCurrentTokenText(dbLink); + } } - isDbLink = false; + return; } + break; + case PERIOD: + signature.append('.'); + connectedIdents = true; + break; + case USING: return; - } - break; - case PERIOD: - signature.append('.'); - connectedIdents = true; - break; - case USING: - return; - default: - if ("@".equals(scanner.text())) { - isDbLink = true; - } - break; - } - } + default: + if ("@".equals(scanner.text())) { + isDbLink = true; + } + break; + } + } } } diff --git a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/signature/SignatureParserTest.java b/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/signature/SignatureParserTest.java index 61c013a215..b64f09fa9f 100644 --- a/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/signature/SignatureParserTest.java +++ b/apm-agent-plugins/apm-jdbc-plugin/src/test/java/co/elastic/apm/agent/jdbc/signature/SignatureParserTest.java @@ -11,9 +11,9 @@ * 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 @@ -50,7 +50,7 @@ void testScanner() { assertThat(getSignature(testCase.get("input").asText())).isEqualTo(testCase.get("output").asText()); } } - + @Test void testDbLinkForUpdate() { final StringBuilder sb = new StringBuilder();