From ac0187bbfda8934d592712a8e3f9bb2e764239dd Mon Sep 17 00:00:00 2001 From: xxia Date: Mon, 25 Oct 2021 11:15:42 -0700 Subject: [PATCH 01/12] obfuscate sql that is above slow query threshold --- .../newrelic/agent/tracers/DefaultSqlTracer.java | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java b/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java index 46ba083d46..c3a79051fb 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java @@ -266,11 +266,15 @@ protected void recordMetrics(TransactionStats transactionStats) { if (isMetricProducer() && getTransaction() != null) { String rawSql = null; Object sqlObject = getSql(); + String appName = getTransaction().getApplicationName(); + if (sqlObject != null) { rawSql = new PreparedStatementSql(sql, params).toString(); + if (slowQuery(appName)) { + rawSql = obfuscateRawSql(rawSql, appName); + } } - String appName = getTransaction().getApplicationName(); String hostToReport = DatastoreMetrics.replaceLocalhost(getHost()); if (getIdentifier() != null) { @@ -301,6 +305,16 @@ protected void recordMetrics(TransactionStats transactionStats) { super.recordMetrics(transactionStats); } + private String obfuscateRawSql(String rawSql, String appName) { + SqlQueryConverter converter = new SqlQueryConverter(appName, getDatabaseVendor()); + return converter.toObfuscatedQueryString(rawSql); + } + + private boolean slowQuery(String appName) { + double threshold = ServiceFactory.getConfigService().getAgentConfig(appName).getTransactionTracerConfig().getExplainThresholdInMillis(); + return getDurationInMilliseconds() > threshold; + } + /** * Returns the sql for the statement instrumented by this driver. This method is overridden by prepared statement * tracers which return a sql object that can delay the rendering of the full sql statement with parameters until it From ae2c7cbf83b96d1afe1f7675642a4624e8ada9b5 Mon Sep 17 00:00:00 2001 From: xi xia Date: Mon, 15 Nov 2021 16:02:17 -0800 Subject: [PATCH 02/12] refactor names --- .../newrelic/agent/tracers/DefaultSqlTracer.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java b/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java index c3a79051fb..eb836c9e14 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java @@ -264,14 +264,14 @@ public boolean isMetricProducer() { @Override protected void recordMetrics(TransactionStats transactionStats) { if (isMetricProducer() && getTransaction() != null) { - String rawSql = null; + String slowQuerySql = null; Object sqlObject = getSql(); String appName = getTransaction().getApplicationName(); if (sqlObject != null) { - rawSql = new PreparedStatementSql(sql, params).toString(); - if (slowQuery(appName)) { - rawSql = obfuscateRawSql(rawSql, appName); + slowQuerySql = new PreparedStatementSql(sql, params).toString(); + if (queryExceedsSlowQueryThreshold(appName)) { + slowQuerySql = obfuscateRawSql(slowQuerySql, appName); } } @@ -284,7 +284,7 @@ protected void recordMetrics(TransactionStats transactionStats) { .operation(parsedDatabaseStatement.getOperation()) .instance(hostToReport, getIdentifier()) .databaseName(getDatabaseName()) - .slowQuery(rawSql, new SqlQueryConverter(appName, getDatabaseVendor())) + .slowQuery(slowQuerySql, new SqlQueryConverter(appName, getDatabaseVendor())) .build()); } else { String portToReport = DatastoreMetrics.replacePort(getPort()); @@ -294,7 +294,7 @@ protected void recordMetrics(TransactionStats transactionStats) { .operation(parsedDatabaseStatement.getOperation()) .instance(hostToReport, portToReport) .databaseName(getDatabaseName()) - .slowQuery(rawSql, new SqlQueryConverter(appName, getDatabaseVendor())) + .slowQuery(slowQuerySql, new SqlQueryConverter(appName, getDatabaseVendor())) .build()); } @@ -310,7 +310,7 @@ private String obfuscateRawSql(String rawSql, String appName) { return converter.toObfuscatedQueryString(rawSql); } - private boolean slowQuery(String appName) { + private boolean queryExceedsSlowQueryThreshold(String appName) { double threshold = ServiceFactory.getConfigService().getAgentConfig(appName).getTransactionTracerConfig().getExplainThresholdInMillis(); return getDurationInMilliseconds() > threshold; } From 2ee648fc9a88cd947054744f1a7046352bbde514 Mon Sep 17 00:00:00 2001 From: xi xia Date: Mon, 15 Nov 2021 16:03:52 -0800 Subject: [PATCH 03/12] clean up warnings --- .../java/com/newrelic/agent/tracers/DefaultSqlTracer.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java b/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java index eb836c9e14..9059d4097c 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java @@ -513,7 +513,7 @@ public String toString() { * @param parameters the parameter map * @return the parameterized SQL */ - public static String parameterizeSql(String sql, Object[] parameters) throws Exception { + public static String parameterizeSql(String sql, Object[] parameters) { if (sql == null || parameters == null || parameters.length == 0) { return sql; } @@ -526,11 +526,11 @@ public static String parameterizeSql(String sql, Object[] parameters) throws Exc } else { Object val = i < parameters.length ? parameters[i] : null; if (val instanceof Number) { - sb.append(piece).append(val.toString()); + sb.append(piece).append(val); } else if (val == null) { sb.append(piece).append("?"); } else { - sb.append(piece).append("'").append(val.toString()).append("'"); + sb.append(piece).append("'").append(val).append("'"); } } } From 39dd747b3925d49917145cff3dd9d93af56fecb0 Mon Sep 17 00:00:00 2001 From: xi xia Date: Mon, 15 Nov 2021 16:40:10 -0800 Subject: [PATCH 04/12] name change --- .../agent/service/analytics/SpanEventFactory.java | 4 ++-- .../agent/sql/DefaultSlowQueryListener.java | 14 +++++++------- .../newrelic/agent/tracers/DefaultSqlTracer.java | 1 - .../com/newrelic/agent/tracers/DefaultTracer.java | 2 +- .../api/agent/SlowQueryDatastoreParameters.java | 8 ++++---- 5 files changed, 14 insertions(+), 15 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/service/analytics/SpanEventFactory.java b/newrelic-agent/src/main/java/com/newrelic/agent/service/analytics/SpanEventFactory.java index ecbc22c9c4..cd9903e5f3 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/service/analytics/SpanEventFactory.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/service/analytics/SpanEventFactory.java @@ -353,9 +353,9 @@ private String determineObfuscationLevel(SlowQueryDatastoreParameters slo if (config.isHighSecurity() || config.getTransactionTracerConfig().getRecordSql().equals(SqlObfuscator.OFF_SETTING)) { return null; } else if (config.getTransactionTracerConfig().getRecordSql().equals(SqlObfuscator.RAW_SETTING)) { - return slowQueryDatastoreParameters.getQueryConverter().toRawQueryString(slowQueryDatastoreParameters.getRawQuery()); + return slowQueryDatastoreParameters.getQueryConverter().toRawQueryString(slowQueryDatastoreParameters.getQuery()); } else { - return slowQueryDatastoreParameters.getQueryConverter().toObfuscatedQueryString(slowQueryDatastoreParameters.getRawQuery()); + return slowQueryDatastoreParameters.getQueryConverter().toObfuscatedQueryString(slowQueryDatastoreParameters.getQuery()); } } diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java b/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java index 448167d8bd..b8304fac7d 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java @@ -43,20 +43,20 @@ public DefaultSlowQueryListener(String appName, double thresholdInMillis) { @Override public void noticeTracer(Tracer tracer, SlowQueryDatastoreParameters slowQueryDatastoreParameters) { if (tracer.getDurationInMilliseconds() > thresholdInMillis) { - T rawQuery = slowQueryDatastoreParameters.getRawQuery(); + T query = slowQueryDatastoreParameters.getQuery(); QueryConverter queryConverter = slowQueryDatastoreParameters.getQueryConverter(); - if (rawQuery == null || queryConverter == null) { + if (query == null || queryConverter == null) { // Ignore tracer return; } - String rawQueryString = queryConverter.toRawQueryString(rawQuery); - if (rawQueryString == null || rawQueryString.trim().isEmpty()) { + String queryString = queryConverter.toRawQueryString(query); + if (queryString == null || queryString.trim().isEmpty()) { // Ignore tracer return; } - String obfuscatedQueryString = queryConverter.toObfuscatedQueryString(rawQuery); + String obfuscatedQueryString = queryConverter.toObfuscatedQueryString(query); if (obfuscatedQueryString == null) { // Ignore tracer if no obfuscated query is provided return; @@ -68,7 +68,7 @@ public void noticeTracer(Tracer tracer, SlowQueryDatastoreParameters slow } // This allows transaction traces to show slow queries directly in the trace details - tracer.setAgentAttribute(SqlTracer.SQL_PARAMETER_NAME, rawQueryString); + tracer.setAgentAttribute(SqlTracer.SQL_PARAMETER_NAME, queryString); tracer.setAgentAttribute(SqlTracer.SQL_OBFUSCATED_PARAMETER_NAME, obfuscatedQueryString); DatastoreConfig datastoreConfig = ServiceFactory.getConfigService().getDefaultAgentConfig().getDatastoreConfig(); @@ -95,7 +95,7 @@ public void noticeTracer(Tracer tracer, SlowQueryDatastoreParameters slow existingInfo.aggregate(tracer); slowQueryInfoCache.putReplace(obfuscatedQueryString, existingInfo); } else { - SlowQueryInfo sqlInfo = new SlowQueryInfo(null, tracer, rawQueryString, obfuscatedQueryString, + SlowQueryInfo sqlInfo = new SlowQueryInfo(null, tracer, queryString, obfuscatedQueryString, tracer.getTransactionActivity().getTransaction().getAgentConfig().getSqlTraceConfig()); sqlInfo.aggregate(tracer); slowQueryInfoCache.putIfAbsent(obfuscatedQueryString, sqlInfo); diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java b/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java index 9059d4097c..67dc2e458b 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java @@ -201,7 +201,6 @@ protected void doFinish(Throwable throwable) { String appName = getTransaction().getApplicationName(); SqlQueryConverter converter = new SqlQueryConverter(appName, getDatabaseVendor()); String obfuscatedQueryString = converter.toObfuscatedQueryString(sql.toString()); - // Store the obfuscated query string getTransaction().getIntrinsicAttributes().put(SQL_PARAMETER_NAME, obfuscatedQueryString); } diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultTracer.java b/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultTracer.java index 2d5dd89b75..b37b4d5df3 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultTracer.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultTracer.java @@ -819,7 +819,7 @@ private void recordMessageBrokerMetrics(MessageConsumeParameters messageConsumeP private void recordSlowQueryData(SlowQueryDatastoreParameters slowQueryDatastoreParameters) { Transaction transaction = getTransactionActivity().getTransaction(); - if (transaction != null && slowQueryDatastoreParameters.getRawQuery() != null + if (transaction != null && slowQueryDatastoreParameters.getQuery() != null && slowQueryDatastoreParameters.getQueryConverter() != null) { // Attempt to record the slow query if it's above the threshold transaction.getSlowQueryListener(true).noticeTracer(this, slowQueryDatastoreParameters); diff --git a/newrelic-api/src/main/java/com/newrelic/api/agent/SlowQueryDatastoreParameters.java b/newrelic-api/src/main/java/com/newrelic/api/agent/SlowQueryDatastoreParameters.java index 231d3950fc..c797a0d41b 100644 --- a/newrelic-api/src/main/java/com/newrelic/api/agent/SlowQueryDatastoreParameters.java +++ b/newrelic-api/src/main/java/com/newrelic/api/agent/SlowQueryDatastoreParameters.java @@ -16,7 +16,7 @@ public class SlowQueryDatastoreParameters extends DatastoreParameters { /** - * The raw query object used for transforming into a raw query String and obfuscated query String + * The query object used for transforming into a raw query String and obfuscated query String */ private final T rawQuery; @@ -39,12 +39,12 @@ protected SlowQueryDatastoreParameters(SlowQueryDatastoreParameters slowQuery } /** - * Returns the raw query object used for processing. + * Returns the query object used for processing. This query might be raw with parameters or obfuscated. * - * @return raw query object + * @return query object * @since 3.36.0 */ - public T getRawQuery() { + public T getQuery() { return rawQuery; } From 9dfa3718cc0d8771ed5a4b7a694c1ed998084390 Mon Sep 17 00:00:00 2001 From: xi xia Date: Tue, 16 Nov 2021 11:16:22 -0800 Subject: [PATCH 05/12] remove setting of raw string on agent attribute --- .../java/com/newrelic/agent/sql/DefaultSlowQueryListener.java | 1 - 1 file changed, 1 deletion(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java b/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java index b8304fac7d..5fc807c4cb 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java @@ -68,7 +68,6 @@ public void noticeTracer(Tracer tracer, SlowQueryDatastoreParameters slow } // This allows transaction traces to show slow queries directly in the trace details - tracer.setAgentAttribute(SqlTracer.SQL_PARAMETER_NAME, queryString); tracer.setAgentAttribute(SqlTracer.SQL_OBFUSCATED_PARAMETER_NAME, obfuscatedQueryString); DatastoreConfig datastoreConfig = ServiceFactory.getConfigService().getDefaultAgentConfig().getDatastoreConfig(); From f456603b78aa74fec10d164f9205f65dfe0d8f17 Mon Sep 17 00:00:00 2001 From: xi xia Date: Wed, 17 Nov 2021 10:49:10 -0800 Subject: [PATCH 06/12] better name and documentation --- .../newrelic/agent/tracers/DefaultSqlTracer.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java b/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java index 67dc2e458b..8e1aa8bfec 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java @@ -270,7 +270,7 @@ protected void recordMetrics(TransactionStats transactionStats) { if (sqlObject != null) { slowQuerySql = new PreparedStatementSql(sql, params).toString(); if (queryExceedsSlowQueryThreshold(appName)) { - slowQuerySql = obfuscateRawSql(slowQuerySql, appName); + slowQuerySql = getNoRawOrObfuscatedSql(slowQuerySql, appName); } } @@ -304,7 +304,16 @@ protected void recordMetrics(TransactionStats transactionStats) { super.recordMetrics(transactionStats); } - private String obfuscateRawSql(String rawSql, String appName) { + /** + * This method is named this way because the docs on toObfuscatedQueryString imply obfuscation certainty. + * The reality is the SqlQueryConverter implementation of this method has introduced conditional + * logic that respects the agent configuration setting `record_sql`. + * + * If OBFUSCATED, sql is obfuscated. + * If RAW, sql is returned, unchanged. + * If OFF, null is returned. + */ + private String getNoRawOrObfuscatedSql(String rawSql, String appName) { SqlQueryConverter converter = new SqlQueryConverter(appName, getDatabaseVendor()); return converter.toObfuscatedQueryString(rawSql); } From 12549ec85ef1f2f8bb9921824dc118591be2cf4c Mon Sep 17 00:00:00 2001 From: xi xia Date: Wed, 17 Nov 2021 15:46:04 -0800 Subject: [PATCH 07/12] WIP address this todo --- .../java/com/newrelic/agent/sql/DefaultSlowQueryListener.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java b/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java index 5fc807c4cb..b491823d16 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java @@ -68,6 +68,8 @@ public void noticeTracer(Tracer tracer, SlowQueryDatastoreParameters slow } // This allows transaction traces to show slow queries directly in the trace details + //todo: obfuscatedQueryString isn't correct, it could be Raw or Obfuscated. The queryConverter + // has conditional obfuscation, respecting recrod_sql setting. tracer.setAgentAttribute(SqlTracer.SQL_OBFUSCATED_PARAMETER_NAME, obfuscatedQueryString); DatastoreConfig datastoreConfig = ServiceFactory.getConfigService().getDefaultAgentConfig().getDatastoreConfig(); From cb010af2a7a8d1e9c2cba5f840c706b0c7a84b9d Mon Sep 17 00:00:00 2001 From: xi xia Date: Wed, 1 Dec 2021 09:27:29 -0800 Subject: [PATCH 08/12] remove unnecessary queryConverter --- .../agent/sql/DefaultSlowQueryListener.java | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java b/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java index b491823d16..cc5f44f9f4 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java @@ -43,25 +43,12 @@ public DefaultSlowQueryListener(String appName, double thresholdInMillis) { @Override public void noticeTracer(Tracer tracer, SlowQueryDatastoreParameters slowQueryDatastoreParameters) { if (tracer.getDurationInMilliseconds() > thresholdInMillis) { - T query = slowQueryDatastoreParameters.getQuery(); - QueryConverter queryConverter = slowQueryDatastoreParameters.getQueryConverter(); - if (query == null || queryConverter == null) { + String query = (String) slowQueryDatastoreParameters.getQuery(); + if (query == null ) { // Ignore tracer return; } - String queryString = queryConverter.toRawQueryString(query); - if (queryString == null || queryString.trim().isEmpty()) { - // Ignore tracer - return; - } - - String obfuscatedQueryString = queryConverter.toObfuscatedQueryString(query); - if (obfuscatedQueryString == null) { - // Ignore tracer if no obfuscated query is provided - return; - } - // Handle an "input query" from an ORM or a framework that automatically generates queries if (slowQueryDatastoreParameters instanceof SlowQueryWithInputDatastoreParameters) { handleInputQuery(tracer, (SlowQueryWithInputDatastoreParameters) slowQueryDatastoreParameters); From e91a27706885f5c222b918e4bcd2d20c8f0d9c85 Mon Sep 17 00:00:00 2001 From: xi xia Date: Wed, 1 Dec 2021 09:28:42 -0800 Subject: [PATCH 09/12] refactor query parameter use --- .../newrelic/agent/sql/DefaultSlowQueryListener.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java b/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java index cc5f44f9f4..7e56688e68 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java @@ -57,7 +57,7 @@ public void noticeTracer(Tracer tracer, SlowQueryDatastoreParameters slow // This allows transaction traces to show slow queries directly in the trace details //todo: obfuscatedQueryString isn't correct, it could be Raw or Obfuscated. The queryConverter // has conditional obfuscation, respecting recrod_sql setting. - tracer.setAgentAttribute(SqlTracer.SQL_OBFUSCATED_PARAMETER_NAME, obfuscatedQueryString); + tracer.setAgentAttribute(SqlTracer.SQL_OBFUSCATED_PARAMETER_NAME, query); DatastoreConfig datastoreConfig = ServiceFactory.getConfigService().getDefaultAgentConfig().getDatastoreConfig(); boolean allUnknown = slowQueryDatastoreParameters.getHost() == null @@ -77,16 +77,16 @@ public void noticeTracer(Tracer tracer, SlowQueryDatastoreParameters slow slowQueryInfoCache = new BoundedConcurrentCache<>(MAX_SQL_TRACERS); } - SlowQueryInfo existingInfo = slowQueryInfoCache.get(obfuscatedQueryString); + SlowQueryInfo existingInfo = slowQueryInfoCache.get(query); if (existingInfo != null) { // Aggregate tracers by SQL. existingInfo.aggregate(tracer); - slowQueryInfoCache.putReplace(obfuscatedQueryString, existingInfo); + slowQueryInfoCache.putReplace(query, existingInfo); } else { - SlowQueryInfo sqlInfo = new SlowQueryInfo(null, tracer, queryString, obfuscatedQueryString, + SlowQueryInfo sqlInfo = new SlowQueryInfo(null, tracer, query, query, tracer.getTransactionActivity().getTransaction().getAgentConfig().getSqlTraceConfig()); sqlInfo.aggregate(tracer); - slowQueryInfoCache.putIfAbsent(obfuscatedQueryString, sqlInfo); + slowQueryInfoCache.putIfAbsent(query, sqlInfo); } } } From 96ccc30a18beed46c05c4448fbfb523e16fc0388 Mon Sep 17 00:00:00 2001 From: xi xia Date: Wed, 1 Dec 2021 14:16:36 -0800 Subject: [PATCH 10/12] remove todo and add comments --- .../agent/sql/DefaultSlowQueryListener.java | 4 ++-- .../newrelic/agent/tracers/DefaultSqlTracer.java | 15 ++++++++------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java b/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java index 7e56688e68..6f2820ce2a 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java @@ -55,8 +55,8 @@ public void noticeTracer(Tracer tracer, SlowQueryDatastoreParameters slow } // This allows transaction traces to show slow queries directly in the trace details - //todo: obfuscatedQueryString isn't correct, it could be Raw or Obfuscated. The queryConverter - // has conditional obfuscation, respecting recrod_sql setting. + // Unfortunately, SQL_OBFUSCATED_PARAMETER_NAME is misleading. Before reaching here, the query has already + // been processed by SqlQueryConverter in DefaultSqlTracer. The result could be Raw, Obfuscated, or null. tracer.setAgentAttribute(SqlTracer.SQL_OBFUSCATED_PARAMETER_NAME, query); DatastoreConfig datastoreConfig = ServiceFactory.getConfigService().getDefaultAgentConfig().getDatastoreConfig(); diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java b/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java index 8e1aa8bfec..e9a60a7b73 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/tracers/DefaultSqlTracer.java @@ -305,13 +305,8 @@ protected void recordMetrics(TransactionStats transactionStats) { } /** - * This method is named this way because the docs on toObfuscatedQueryString imply obfuscation certainty. - * The reality is the SqlQueryConverter implementation of this method has introduced conditional - * logic that respects the agent configuration setting `record_sql`. - * - * If OBFUSCATED, sql is obfuscated. - * If RAW, sql is returned, unchanged. - * If OFF, null is returned. + * This method is named this way because {@link SqlQueryConverter#toObfuscatedQueryString(String)} has + * unexpected conditional logic. */ private String getNoRawOrObfuscatedSql(String rawSql, String appName) { SqlQueryConverter converter = new SqlQueryConverter(appName, getDatabaseVendor()); @@ -559,6 +554,12 @@ public String toRawQueryString(String rawQuery) { return rawQuery; } + /** + * For this implementation, the getSqlfuscator has conditional + * logic to return an obfuscator that respects the agent configuration setting `record_sql` + * + * See {@link com.newrelic.agent.database.DatabaseService#createSqlObfuscator(TransactionTracerConfig) } + */ @Override public String toObfuscatedQueryString(String rawQuery) { SqlObfuscator sqlObfuscator = ServiceFactory.getDatabaseService().getSqlObfuscator(appName); From 8cc788b3627515377cc76b881157aed71a40640a Mon Sep 17 00:00:00 2001 From: Andre Onuki Date: Thu, 6 Jan 2022 11:36:16 -0500 Subject: [PATCH 11/12] Setting correct attribute name for the sql in tracer attrs --- .../test/java/test/newrelic/test/agent/api/ApiTest.java | 1 - .../com/newrelic/agent/sql/DefaultSlowQueryListener.java | 9 +++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/functional_test/src/test/java/test/newrelic/test/agent/api/ApiTest.java b/functional_test/src/test/java/test/newrelic/test/agent/api/ApiTest.java index 30530ff1f7..d5a34fb5a6 100644 --- a/functional_test/src/test/java/test/newrelic/test/agent/api/ApiTest.java +++ b/functional_test/src/test/java/test/newrelic/test/agent/api/ApiTest.java @@ -2039,7 +2039,6 @@ public String toObfuscatedQueryString(BsonDocument query) { Map attributes = rootTracer.getAgentAttributes(); Assert.assertNotNull(attributes.get("sql")); - Assert.assertNotNull(attributes.get("sql_obfuscated")); } /* Messaging - FIT to Public API */ diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java b/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java index 6f2820ce2a..cb19932d49 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/sql/DefaultSlowQueryListener.java @@ -43,7 +43,7 @@ public DefaultSlowQueryListener(String appName, double thresholdInMillis) { @Override public void noticeTracer(Tracer tracer, SlowQueryDatastoreParameters slowQueryDatastoreParameters) { if (tracer.getDurationInMilliseconds() > thresholdInMillis) { - String query = (String) slowQueryDatastoreParameters.getQuery(); + String query = slowQueryDatastoreParameters.getQuery().toString(); if (query == null ) { // Ignore tracer return; @@ -54,10 +54,11 @@ public void noticeTracer(Tracer tracer, SlowQueryDatastoreParameters slow handleInputQuery(tracer, (SlowQueryWithInputDatastoreParameters) slowQueryDatastoreParameters); } + String recordSql = ServiceFactory.getConfigService().getDefaultAgentConfig().getTransactionTracerConfig().getRecordSql(); + String sqlAttr = SqlObfuscator.RAW_SETTING.equals(recordSql) ? + SqlTracer.SQL_PARAMETER_NAME : SqlTracer.SQL_OBFUSCATED_PARAMETER_NAME; // This allows transaction traces to show slow queries directly in the trace details - // Unfortunately, SQL_OBFUSCATED_PARAMETER_NAME is misleading. Before reaching here, the query has already - // been processed by SqlQueryConverter in DefaultSqlTracer. The result could be Raw, Obfuscated, or null. - tracer.setAgentAttribute(SqlTracer.SQL_OBFUSCATED_PARAMETER_NAME, query); + tracer.setAgentAttribute(sqlAttr, query); DatastoreConfig datastoreConfig = ServiceFactory.getConfigService().getDefaultAgentConfig().getDatastoreConfig(); boolean allUnknown = slowQueryDatastoreParameters.getHost() == null From afc1ab4311535cc133330643e5a5cec2341ae212 Mon Sep 17 00:00:00 2001 From: Todd W Crone Date: Thu, 6 Jan 2022 17:37:57 -0500 Subject: [PATCH 12/12] Switch to Gradle Github Action --- .github/workflows/GHA-Functional-Tests.yaml | 4 +++- .github/workflows/GHA-Scala-Functional-Tests.yaml | 4 +++- .github/workflows/GHA-Scala-Instrumentation-Tests.yaml | 4 +++- .github/workflows/GHA-Unit-Tests.yaml | 4 +++- .github/workflows/X-Reusable-Test.yml | 4 +++- 5 files changed, 15 insertions(+), 5 deletions(-) diff --git a/.github/workflows/GHA-Functional-Tests.yaml b/.github/workflows/GHA-Functional-Tests.yaml index 74d205b996..5c34c266c9 100644 --- a/.github/workflows/GHA-Functional-Tests.yaml +++ b/.github/workflows/GHA-Functional-Tests.yaml @@ -38,7 +38,6 @@ jobs: with: distribution: 'adopt' java-version: 8 - cache: gradle - name: Save JAVA_HOME as JDK8 for later usage run: | @@ -79,6 +78,9 @@ jobs: rm gradle.properties mv gradle.properties.gha gradle.properties + - name: Setup Gradle + uses: gradle/gradle-build-action@v2 + - name: Build newrelicJar if: ${{ env.GRADLE_KEY != '' }} env: diff --git a/.github/workflows/GHA-Scala-Functional-Tests.yaml b/.github/workflows/GHA-Scala-Functional-Tests.yaml index 2515504c5e..bfd736d244 100644 --- a/.github/workflows/GHA-Scala-Functional-Tests.yaml +++ b/.github/workflows/GHA-Scala-Functional-Tests.yaml @@ -38,7 +38,6 @@ jobs: with: distribution: 'adopt' java-version: 8 - cache: gradle - name: Save JAVA_HOME as JDK8 for later usage run: | @@ -67,6 +66,9 @@ jobs: rm gradle.properties mv gradle.properties.gha gradle.properties + - name: Setup Gradle + uses: gradle/gradle-build-action@v2 + - name: Build newrelicJar if: ${{ env.GRADLE_KEY != '' }} env: diff --git a/.github/workflows/GHA-Scala-Instrumentation-Tests.yaml b/.github/workflows/GHA-Scala-Instrumentation-Tests.yaml index d3456c1e1d..f236da2023 100644 --- a/.github/workflows/GHA-Scala-Instrumentation-Tests.yaml +++ b/.github/workflows/GHA-Scala-Instrumentation-Tests.yaml @@ -45,7 +45,6 @@ jobs: with: distribution: 'adopt' java-version: 8 - cache: gradle - name: Save JAVA_HOME as JDK8 for later usage run: | @@ -90,6 +89,9 @@ jobs: run: find instrumentation -name "*.jar" ## End AWS jars - plan to cache (check for cache, restore if required) + - name: Setup Gradle + uses: gradle/gradle-build-action@v2 + - name: Build newrelicJar if: ${{ env.GRADLE_KEY != '' }} env: diff --git a/.github/workflows/GHA-Unit-Tests.yaml b/.github/workflows/GHA-Unit-Tests.yaml index bd18841a09..4601f21a65 100644 --- a/.github/workflows/GHA-Unit-Tests.yaml +++ b/.github/workflows/GHA-Unit-Tests.yaml @@ -39,7 +39,6 @@ jobs: with: distribution: 'adopt' java-version: 8 - cache: 'gradle' - name: Save JAVA_HOME as JDK8 for later usage run: | @@ -80,6 +79,9 @@ jobs: rm gradle.properties mv gradle.properties.gha gradle.properties + - name: Setup Gradle + uses: gradle/gradle-build-action@v2 + - name: Run unit tests for each Java version as defined in the matrix if: ${{ env.GRADLE_KEY != '' }} env: diff --git a/.github/workflows/X-Reusable-Test.yml b/.github/workflows/X-Reusable-Test.yml index b8114b1061..9f6eabb078 100644 --- a/.github/workflows/X-Reusable-Test.yml +++ b/.github/workflows/X-Reusable-Test.yml @@ -41,7 +41,6 @@ jobs: with: distribution: 'adopt' java-version: 8 - cache: 'gradle' # Save new JDK variable - name: Save JAVA_HOME as JDK8 for later usage @@ -101,6 +100,9 @@ jobs: run: find instrumentation -name "*.jar" ## End AWS jars - plan to cache (check for cache, restore if required) + - name: Setup Gradle + uses: gradle/gradle-build-action@v2 + # Is newrelicJar present in newrelic-agent/build # TO DO: Below version number has to be dynamic - name: Build newrelicJar