You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jackrabbit.apache.org by GitBox <gi...@apache.org> on 2021/11/18 08:50:07 UTC
[GitHub] [jackrabbit-oak] anchela commented on a change in pull request #419: OAK-9624 add query caller logging
anchela commented on a change in pull request #419:
URL: https://github.com/apache/jackrabbit-oak/pull/419#discussion_r751989450
##########
File path: oak-api/src/main/java/org/apache/jackrabbit/oak/api/jmx/QueryEngineSettingsMBean.java
##########
@@ -139,5 +139,14 @@ void setQueryValidatorPattern(
@Description("Get the query validator data as a JSON string.")
String getQueryValidatorJson();
-
+
+
+// @Description("Set or remove Java Package Name to ignore in Call Trace analysis")
+ void setIgnoredClassNamesinCallTrace(
+ @Description("package names")
Review comment:
see above.... maybe you have to remove it here....?
##########
File path: oak-api/src/main/java/org/apache/jackrabbit/oak/api/jmx/QueryEngineSettingsMBean.java
##########
@@ -139,5 +139,14 @@ void setQueryValidatorPattern(
@Description("Get the query validator data as a JSON string.")
String getQueryValidatorJson();
-
+
+
+// @Description("Set or remove Java Package Name to ignore in Call Trace analysis")
Review comment:
why do you comment this?
if i look at other methods, the '@Description' tag is above the method with some human readable information.
##########
File path: oak-api/src/main/java/org/apache/jackrabbit/oak/api/jmx/QueryEngineSettingsMBean.java
##########
@@ -139,5 +139,14 @@ void setQueryValidatorPattern(
@Description("Get the query validator data as a JSON string.")
String getQueryValidatorJson();
-
+
+
+// @Description("Set or remove Java Package Name to ignore in Call Trace analysis")
+ void setIgnoredClassNamesinCallTrace(
+ @Description("package names")
+ @Name("package names")
+ String[] packageNames);
Review comment:
i would add @notnull annotation
##########
File path: oak-api/src/main/java/org/apache/jackrabbit/oak/api/jmx/QueryEngineSettingsMBean.java
##########
@@ -139,5 +139,14 @@ void setQueryValidatorPattern(
@Description("Get the query validator data as a JSON string.")
String getQueryValidatorJson();
-
+
+
+// @Description("Set or remove Java Package Name to ignore in Call Trace analysis")
+ void setIgnoredClassNamesinCallTrace(
+ @Description("package names")
+ @Name("package names")
+ String[] packageNames);
+
+// @Description("Get the Java package Names to ignore in Call trace analysis")
+ String[] getIgnoredClassNamesinCallTrace();
Review comment:
i would add @Notnull annotation for the return value. that helps simplifying the code and makes the contract clear. consumers with the right IDE settings will be notified if they add redundant checks for null.
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java
##########
@@ -182,6 +186,14 @@ public String getQueryValidatorJson() {
public QueryValidator getQueryValidator() {
return queryValidator;
}
+
+ public void setIgnoredClassNamesinCallTrace(String[] packageNames) {
+ classNamesIgnoredInCallTrace = packageNames;
Review comment:
with the annotation you can avoid having to check for null param here.... which would overwrite the default empty-string value..... i like the code in this form and i would avoid redundant checks for null by adding the annotation.
##########
File path: oak-api/src/main/java/org/apache/jackrabbit/oak/api/jmx/QueryEngineSettingsMBean.java
##########
@@ -139,5 +139,14 @@ void setQueryValidatorPattern(
@Description("Get the query validator data as a JSON string.")
String getQueryValidatorJson();
-
+
+
+// @Description("Set or remove Java Package Name to ignore in Call Trace analysis")
+ void setIgnoredClassNamesinCallTrace(
+ @Description("package names")
+ @Name("package names")
+ String[] packageNames);
+
+// @Description("Get the Java package Names to ignore in Call trace analysis")
Review comment:
same as on line 144
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryInformationLogger.java
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law
+ * or agreed to in writing, software distributed under the License is
+ * distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the specific language
+ * governing permissions and limitations under the License.
+ */
+package org.apache.jackrabbit.oak.query;
+
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class QueryInformationLogger {
+
+ private static final Logger INTERNAL_QUERIES_LOG = LoggerFactory.getLogger(QueryInformationLogger.class.getName() + ".internalQuery");
+ private static final Logger EXTERNAL_QUERIES_LOG = LoggerFactory.getLogger(QueryInformationLogger.class.getName() + ".externalQuery");
+
+ protected static final String OAK_INTERNAL_MARKER = "/* oak-internal */";
+
+ /**
+ * Logs the caller of the query based on the callstack. To refine the result, all stack frames
+ * are ignored, for which the entry (consisting of package name, classname and method name) starts
+ * with one of the entries provided by ignoredClassNames.
+ *
+ * @param statement the query statement
+ * @param ignoredClassNames entries to be ignored. If empty, the method is a no-op.
+ */
+ public static void logCaller (String statement, String[] ignoredClassNames) {
+
+ if (ignoredClassNames.length == 0) {
Review comment:
:) that one would result in NPE if the 'ignoredClassNames' array were to be null..... that's why i like the notnull annotations.
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java
##########
@@ -182,6 +186,14 @@ public String getQueryValidatorJson() {
public QueryValidator getQueryValidator() {
return queryValidator;
}
+
+ public void setIgnoredClassNamesinCallTrace(String[] packageNames) {
Review comment:
add @notnull annotations to the param
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettingsService.java
##########
@@ -77,6 +77,14 @@
"the queryPaths of the index is taken into account."
)
String getStrictPathRestrictionsForIndexes() default DISABLED_STRICT_PATH_RESTRICTION;
+
+ @AttributeDefinition(
+ name="Fully qualified class names to ignore when finding caller",
+ description="If non-empty the query engine logs the query statement plus the java package "
Review comment:
if you prefer not to add the notnull annotation you should explain what a null return value means..... but i would suggest to avoid return null....
##########
File path: oak-core/src/test/java/org/apache/jackrabbit/oak/query/QueryInformationLoggerTest.java
##########
@@ -0,0 +1,143 @@
+package org.apache.jackrabbit.oak.query;
+
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.jackrabbit.oak.commons.junit.LogCustomizer;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.event.Level;
+
+public class QueryInformationLoggerTest {
+
+ private static final Logger LOG = LoggerFactory.getLogger(QueryInformationLoggerTest.class);
+
+ private static final String EXTERNAL_LOG = QueryInformationLogger.class.getName() + ".externalQuery";
+ private static final String INTERNAL_LOG = QueryInformationLogger.class.getName() + ".internalQuery";
+
+ String[] ignoredClasses = new String[] {"org.apache.jackrabbit.oak","java.lang","sun.reflect", "jdk"};
+ String[] allClassesIgnored = new String[] {"java","org","net","sun","jdk"};
+
+
+ @Test
+ public void regularQueryTest() {
+
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.INFO).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+ assertEquals("Exactly 1 entry must be written",1,external.getLogs().size());
+ String logEntry = external.getLogs().get(0);
+ assertTrue(logEntry.contains("org.junit.runners.model")); // org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void withStacktrace() {
+
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+ assertEquals("Exactly 2 entries must be written",2,external.getLogs().size()); //
+ assertEquals(0,internal.getLogs().size());
Review comment:
formatting: add a space after the ,
##########
File path: oak-core/src/test/java/org/apache/jackrabbit/oak/query/QueryInformationLoggerTest.java
##########
@@ -0,0 +1,143 @@
+package org.apache.jackrabbit.oak.query;
+
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.jackrabbit.oak.commons.junit.LogCustomizer;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.event.Level;
+
+public class QueryInformationLoggerTest {
+
+ private static final Logger LOG = LoggerFactory.getLogger(QueryInformationLoggerTest.class);
+
+ private static final String EXTERNAL_LOG = QueryInformationLogger.class.getName() + ".externalQuery";
+ private static final String INTERNAL_LOG = QueryInformationLogger.class.getName() + ".internalQuery";
+
+ String[] ignoredClasses = new String[] {"org.apache.jackrabbit.oak","java.lang","sun.reflect", "jdk"};
+ String[] allClassesIgnored = new String[] {"java","org","net","sun","jdk"};
+
+
+ @Test
+ public void regularQueryTest() {
+
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.INFO).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+ assertEquals("Exactly 1 entry must be written",1,external.getLogs().size());
+ String logEntry = external.getLogs().get(0);
+ assertTrue(logEntry.contains("org.junit.runners.model")); // org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void withStacktrace() {
+
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+ assertEquals("Exactly 2 entries must be written",2,external.getLogs().size()); //
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void testOakInternalStatement() {
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.INFO).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page] " + QueryInformationLogger.OAK_INTERNAL_MARKER;
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+
+ // On INFO no logs are written
+ assertEquals(0,external.getLogs().size()); //
Review comment:
- formatting: add a space after ,
- i think the // at the end of the line are not needed, right? the comment is already above
##########
File path: oak-core/src/test/java/org/apache/jackrabbit/oak/query/QueryInformationLoggerTest.java
##########
@@ -0,0 +1,143 @@
+package org.apache.jackrabbit.oak.query;
+
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.jackrabbit.oak.commons.junit.LogCustomizer;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.event.Level;
+
+public class QueryInformationLoggerTest {
+
+ private static final Logger LOG = LoggerFactory.getLogger(QueryInformationLoggerTest.class);
+
+ private static final String EXTERNAL_LOG = QueryInformationLogger.class.getName() + ".externalQuery";
+ private static final String INTERNAL_LOG = QueryInformationLogger.class.getName() + ".internalQuery";
+
+ String[] ignoredClasses = new String[] {"org.apache.jackrabbit.oak","java.lang","sun.reflect", "jdk"};
+ String[] allClassesIgnored = new String[] {"java","org","net","sun","jdk"};
+
+
+ @Test
+ public void regularQueryTest() {
+
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.INFO).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+ assertEquals("Exactly 1 entry must be written",1,external.getLogs().size());
Review comment:
formatting: i would add spaces after each , for the sake of readability
##########
File path: oak-core/src/test/java/org/apache/jackrabbit/oak/query/QueryInformationLoggerTest.java
##########
@@ -0,0 +1,143 @@
+package org.apache.jackrabbit.oak.query;
+
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.jackrabbit.oak.commons.junit.LogCustomizer;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.event.Level;
+
+public class QueryInformationLoggerTest {
+
+ private static final Logger LOG = LoggerFactory.getLogger(QueryInformationLoggerTest.class);
+
+ private static final String EXTERNAL_LOG = QueryInformationLogger.class.getName() + ".externalQuery";
+ private static final String INTERNAL_LOG = QueryInformationLogger.class.getName() + ".internalQuery";
+
+ String[] ignoredClasses = new String[] {"org.apache.jackrabbit.oak","java.lang","sun.reflect", "jdk"};
+ String[] allClassesIgnored = new String[] {"java","org","net","sun","jdk"};
+
+
+ @Test
+ public void regularQueryTest() {
+
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.INFO).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+ assertEquals("Exactly 1 entry must be written",1,external.getLogs().size());
+ String logEntry = external.getLogs().get(0);
+ assertTrue(logEntry.contains("org.junit.runners.model")); // org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void withStacktrace() {
+
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+ assertEquals("Exactly 2 entries must be written",2,external.getLogs().size()); //
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void testOakInternalStatement() {
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.INFO).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page] " + QueryInformationLogger.OAK_INTERNAL_MARKER;
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+
+ // On INFO no logs are written
+ assertEquals(0,external.getLogs().size()); //
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void testWithAllStackEntriesIgnored_INFO() {
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.INFO).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, allClassesIgnored);
+ try {
+ throw new AssertionError();
+ } catch (AssertionError e) {
+ e.printStackTrace();
+ }
+
+ assertEquals(0,external.getLogs().size());
+ assertEquals(0,internal.getLogs().size()); // would require DEBUG
Review comment:
- formatting again :)
- you could simplify as assertTrue(external.getLogs().isEmpty().. but that's probably a matter of taste.
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/Oak.java
##########
@@ -974,6 +976,15 @@ public String toString() {
void setFullTextComparisonWithoutIndex(boolean fullTextComparisonWithoutIndex) {
this.settings.setFullTextComparisonWithoutIndex(fullTextComparisonWithoutIndex);
}
+
+ public void setIgnoredClassNamesinCallTrace(String[] packageNames) {
Review comment:
also here: @notnull annotation to the param
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java
##########
@@ -182,6 +186,14 @@ public String getQueryValidatorJson() {
public QueryValidator getQueryValidator() {
return queryValidator;
}
+
+ public void setIgnoredClassNamesinCallTrace(String[] packageNames) {
+ classNamesIgnoredInCallTrace = packageNames;
+ }
+
+ public String[] getIgnoredClassNamesinCallTrace() {
Review comment:
@notnull return value annotation
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/Oak.java
##########
@@ -60,6 +60,8 @@
import org.apache.jackrabbit.oak.api.ContentSession;
import org.apache.jackrabbit.oak.api.Descriptors;
import org.apache.jackrabbit.oak.api.Root;
+import org.apache.jackrabbit.oak.api.jmx.Description;
+import org.apache.jackrabbit.oak.api.jmx.Name;
Review comment:
why are these 2 imports needed? i didn't look at it in an IDE.... so i might be mistaken..... but i don't see these 2 imports used in the 2 methods added below.
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryInformationLogger.java
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law
+ * or agreed to in writing, software distributed under the License is
+ * distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the specific language
+ * governing permissions and limitations under the License.
+ */
+package org.apache.jackrabbit.oak.query;
+
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class QueryInformationLogger {
+
+ private static final Logger INTERNAL_QUERIES_LOG = LoggerFactory.getLogger(QueryInformationLogger.class.getName() + ".internalQuery");
+ private static final Logger EXTERNAL_QUERIES_LOG = LoggerFactory.getLogger(QueryInformationLogger.class.getName() + ".externalQuery");
+
+ protected static final String OAK_INTERNAL_MARKER = "/* oak-internal */";
Review comment:
that /* oak-internal */ is no specific for the QueryInformationLogger, right?
so, i would rather have it present with some generic IndexConstants object...... e.g. org.apache.jackrabbit.oak.plugins.index.IndexConstants. would that work?
also i don't like too much that it's protected.... can't explain just a gut feeling :)
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java
##########
@@ -80,6 +82,8 @@
private StrictPathRestriction strictPathRestriction = StrictPathRestriction.DISABLE;
private final QueryStatsMBeanImpl queryStats = new QueryStatsMBeanImpl(this);
+
+ private String[] classNamesIgnoredInCallTrace = new String[0];
Review comment:
sounds like a reasonable default..... and when reading it i noticed about the missing @nonull annotations.... because the default might get replaced by 'null' upon set if you don't specify it to be not-null.....
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/Oak.java
##########
@@ -974,6 +976,15 @@ public String toString() {
void setFullTextComparisonWithoutIndex(boolean fullTextComparisonWithoutIndex) {
this.settings.setFullTextComparisonWithoutIndex(fullTextComparisonWithoutIndex);
}
+
+ public void setIgnoredClassNamesinCallTrace(String[] packageNames) {
+ settings.setIgnoredClassNamesinCallTrace(packageNames);
+ }
+
+ public String[] getIgnoredClassNamesinCallTrace() {
Review comment:
add @notnull annotation for the return value
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryInformationLogger.java
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law
+ * or agreed to in writing, software distributed under the License is
+ * distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the specific language
+ * governing permissions and limitations under the License.
+ */
+package org.apache.jackrabbit.oak.query;
+
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class QueryInformationLogger {
Review comment:
as far as i can see the QueryInformationLogger only has static methods and static fields.
so, it's a utility right? IMHO it should in this case come with a private constructor that avoids having instances created.
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryInformationLogger.java
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law
+ * or agreed to in writing, software distributed under the License is
+ * distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the specific language
+ * governing permissions and limitations under the License.
+ */
+package org.apache.jackrabbit.oak.query;
+
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class QueryInformationLogger {
+
+ private static final Logger INTERNAL_QUERIES_LOG = LoggerFactory.getLogger(QueryInformationLogger.class.getName() + ".internalQuery");
+ private static final Logger EXTERNAL_QUERIES_LOG = LoggerFactory.getLogger(QueryInformationLogger.class.getName() + ".externalQuery");
+
+ protected static final String OAK_INTERNAL_MARKER = "/* oak-internal */";
+
+ /**
+ * Logs the caller of the query based on the callstack. To refine the result, all stack frames
+ * are ignored, for which the entry (consisting of package name, classname and method name) starts
+ * with one of the entries provided by ignoredClassNames.
+ *
+ * @param statement the query statement
+ * @param ignoredClassNames entries to be ignored. If empty, the method is a no-op.
+ */
+ public static void logCaller (String statement, String[] ignoredClassNames) {
+
+ if (ignoredClassNames.length == 0) {
+ return;
+ }
+
+ if (isOakInternalQuery(statement)) {
+ INTERNAL_QUERIES_LOG.debug("Oak-internal query, query=[{}]", statement);
+ return;
+ }
+
+ String callingClass = getInvokingClass (ignoredClassNames);
+ if (callingClass.isEmpty()) {
+ INTERNAL_QUERIES_LOG.debug("Oak-internal query, query=[{}]", statement);
Review comment:
why is it an internal query if the callingClass is empty?
for me as an outsider that feels a bit odd.... maybe add a comment explaining it?
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettingsService.java
##########
@@ -77,6 +77,14 @@
"the queryPaths of the index is taken into account."
)
String getStrictPathRestrictionsForIndexes() default DISABLED_STRICT_PATH_RESTRICTION;
+
+ @AttributeDefinition(
+ name="Fully qualified class names to ignore when finding caller",
+ description="If non-empty the query engine logs the query statement plus the java package "
+ + "which executed this query. This java package is the first package in the call trace"
+ + "which does not not start with the any of the provided fully qualfied class names (packagename + classname)"
+ )
+ String[] ignoredClassNamesinCallTrace();
Review comment:
@notnull annotation for the return value :)
##########
File path: oak-core/src/test/java/org/apache/jackrabbit/oak/query/QueryInformationLoggerTest.java
##########
@@ -0,0 +1,143 @@
+package org.apache.jackrabbit.oak.query;
+
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.jackrabbit.oak.commons.junit.LogCustomizer;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.event.Level;
+
+public class QueryInformationLoggerTest {
+
+ private static final Logger LOG = LoggerFactory.getLogger(QueryInformationLoggerTest.class);
+
+ private static final String EXTERNAL_LOG = QueryInformationLogger.class.getName() + ".externalQuery";
+ private static final String INTERNAL_LOG = QueryInformationLogger.class.getName() + ".internalQuery";
+
+ String[] ignoredClasses = new String[] {"org.apache.jackrabbit.oak","java.lang","sun.reflect", "jdk"};
+ String[] allClassesIgnored = new String[] {"java","org","net","sun","jdk"};
+
+
+ @Test
+ public void regularQueryTest() {
+
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.INFO).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+ assertEquals("Exactly 1 entry must be written",1,external.getLogs().size());
+ String logEntry = external.getLogs().get(0);
+ assertTrue(logEntry.contains("org.junit.runners.model")); // org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
+ assertEquals(0,internal.getLogs().size());
Review comment:
formatting: i would add a space after the ,
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryInformationLogger.java
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law
+ * or agreed to in writing, software distributed under the License is
+ * distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the specific language
+ * governing permissions and limitations under the License.
+ */
+package org.apache.jackrabbit.oak.query;
+
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class QueryInformationLogger {
+
+ private static final Logger INTERNAL_QUERIES_LOG = LoggerFactory.getLogger(QueryInformationLogger.class.getName() + ".internalQuery");
+ private static final Logger EXTERNAL_QUERIES_LOG = LoggerFactory.getLogger(QueryInformationLogger.class.getName() + ".externalQuery");
+
+ protected static final String OAK_INTERNAL_MARKER = "/* oak-internal */";
+
+ /**
+ * Logs the caller of the query based on the callstack. To refine the result, all stack frames
+ * are ignored, for which the entry (consisting of package name, classname and method name) starts
+ * with one of the entries provided by ignoredClassNames.
+ *
+ * @param statement the query statement
+ * @param ignoredClassNames entries to be ignored. If empty, the method is a no-op.
+ */
+ public static void logCaller (String statement, String[] ignoredClassNames) {
+
+ if (ignoredClassNames.length == 0) {
+ return;
+ }
+
+ if (isOakInternalQuery(statement)) {
+ INTERNAL_QUERIES_LOG.debug("Oak-internal query, query=[{}]", statement);
+ return;
+ }
+
+ String callingClass = getInvokingClass (ignoredClassNames);
+ if (callingClass.isEmpty()) {
+ INTERNAL_QUERIES_LOG.debug("Oak-internal query, query=[{}]", statement);
+ if (INTERNAL_QUERIES_LOG.isTraceEnabled()) {
+ try {
+ throw new RuntimeException("requested stacktrace");
+ } catch (RuntimeException e) {
+ INTERNAL_QUERIES_LOG.trace("providing requested stacktrace", e);
+ }
+ }
+ } else {
+ EXTERNAL_QUERIES_LOG.info("query=[{}], caller=[{}]",statement, callingClass);
+ if (EXTERNAL_QUERIES_LOG.isDebugEnabled()) {
+ try {
+ throw new RuntimeException("requested stacktrace");
+ } catch (RuntimeException e) {
+ EXTERNAL_QUERIES_LOG.debug("providing requested stacktrace", e);
+ }
+ }
+ }
+ }
+
+ /**
+ * Retrieves the calling class and method from the call stack; this is determined by unwinding
+ * the stack until it finds a combination of full qualified classname + method (separated by ".") which
+ * do not start with any of the values provided by the ignoredJavaPackages parameters.
+ *
+ * @param ignoredJavaPackages
+ * @return the calling class; if all classes of the call trace are in packages which are ignored,
+ * an empty string is returned.
+ */
+ private static String getInvokingClass (String[] ignoredJavaPackages) {
Review comment:
- add @notnull annotations for param and return value
- formatting: extra whitespace between method name and (
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryInformationLogger.java
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law
+ * or agreed to in writing, software distributed under the License is
+ * distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the specific language
+ * governing permissions and limitations under the License.
+ */
+package org.apache.jackrabbit.oak.query;
+
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class QueryInformationLogger {
+
+ private static final Logger INTERNAL_QUERIES_LOG = LoggerFactory.getLogger(QueryInformationLogger.class.getName() + ".internalQuery");
+ private static final Logger EXTERNAL_QUERIES_LOG = LoggerFactory.getLogger(QueryInformationLogger.class.getName() + ".externalQuery");
+
+ protected static final String OAK_INTERNAL_MARKER = "/* oak-internal */";
+
+ /**
+ * Logs the caller of the query based on the callstack. To refine the result, all stack frames
+ * are ignored, for which the entry (consisting of package name, classname and method name) starts
+ * with one of the entries provided by ignoredClassNames.
+ *
+ * @param statement the query statement
+ * @param ignoredClassNames entries to be ignored. If empty, the method is a no-op.
+ */
+ public static void logCaller (String statement, String[] ignoredClassNames) {
+
+ if (ignoredClassNames.length == 0) {
+ return;
+ }
+
+ if (isOakInternalQuery(statement)) {
+ INTERNAL_QUERIES_LOG.debug("Oak-internal query, query=[{}]", statement);
+ return;
+ }
+
+ String callingClass = getInvokingClass (ignoredClassNames);
Review comment:
formatting: extra white space between getInvokingClass and (
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryInformationLogger.java
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law
+ * or agreed to in writing, software distributed under the License is
+ * distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the specific language
+ * governing permissions and limitations under the License.
+ */
+package org.apache.jackrabbit.oak.query;
+
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class QueryInformationLogger {
+
+ private static final Logger INTERNAL_QUERIES_LOG = LoggerFactory.getLogger(QueryInformationLogger.class.getName() + ".internalQuery");
+ private static final Logger EXTERNAL_QUERIES_LOG = LoggerFactory.getLogger(QueryInformationLogger.class.getName() + ".externalQuery");
+
+ protected static final String OAK_INTERNAL_MARKER = "/* oak-internal */";
+
+ /**
+ * Logs the caller of the query based on the callstack. To refine the result, all stack frames
+ * are ignored, for which the entry (consisting of package name, classname and method name) starts
+ * with one of the entries provided by ignoredClassNames.
+ *
+ * @param statement the query statement
+ * @param ignoredClassNames entries to be ignored. If empty, the method is a no-op.
+ */
+ public static void logCaller (String statement, String[] ignoredClassNames) {
Review comment:
i would again add @notnull annotations for the 2 params... otherwise you have to explain what a null value means and how it is handled. so far you only explain what an empty string means....
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryInformationLogger.java
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law
+ * or agreed to in writing, software distributed under the License is
+ * distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the specific language
+ * governing permissions and limitations under the License.
+ */
+package org.apache.jackrabbit.oak.query;
+
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class QueryInformationLogger {
+
+ private static final Logger INTERNAL_QUERIES_LOG = LoggerFactory.getLogger(QueryInformationLogger.class.getName() + ".internalQuery");
+ private static final Logger EXTERNAL_QUERIES_LOG = LoggerFactory.getLogger(QueryInformationLogger.class.getName() + ".externalQuery");
+
+ protected static final String OAK_INTERNAL_MARKER = "/* oak-internal */";
+
+ /**
+ * Logs the caller of the query based on the callstack. To refine the result, all stack frames
+ * are ignored, for which the entry (consisting of package name, classname and method name) starts
+ * with one of the entries provided by ignoredClassNames.
+ *
+ * @param statement the query statement
+ * @param ignoredClassNames entries to be ignored. If empty, the method is a no-op.
+ */
+ public static void logCaller (String statement, String[] ignoredClassNames) {
+
+ if (ignoredClassNames.length == 0) {
+ return;
+ }
+
+ if (isOakInternalQuery(statement)) {
+ INTERNAL_QUERIES_LOG.debug("Oak-internal query, query=[{}]", statement);
+ return;
+ }
+
+ String callingClass = getInvokingClass (ignoredClassNames);
+ if (callingClass.isEmpty()) {
+ INTERNAL_QUERIES_LOG.debug("Oak-internal query, query=[{}]", statement);
+ if (INTERNAL_QUERIES_LOG.isTraceEnabled()) {
+ try {
+ throw new RuntimeException("requested stacktrace");
+ } catch (RuntimeException e) {
+ INTERNAL_QUERIES_LOG.trace("providing requested stacktrace", e);
+ }
+ }
+ } else {
+ EXTERNAL_QUERIES_LOG.info("query=[{}], caller=[{}]",statement, callingClass);
+ if (EXTERNAL_QUERIES_LOG.isDebugEnabled()) {
+ try {
+ throw new RuntimeException("requested stacktrace");
+ } catch (RuntimeException e) {
+ EXTERNAL_QUERIES_LOG.debug("providing requested stacktrace", e);
+ }
+ }
+ }
+ }
+
+ /**
+ * Retrieves the calling class and method from the call stack; this is determined by unwinding
+ * the stack until it finds a combination of full qualified classname + method (separated by ".") which
+ * do not start with any of the values provided by the ignoredJavaPackages parameters.
+ *
+ * @param ignoredJavaPackages
+ * @return the calling class; if all classes of the call trace are in packages which are ignored,
+ * an empty string is returned.
+ */
+ private static String getInvokingClass (String[] ignoredJavaPackages) {
+
+ // With java9 we would use https://docs.oracle.com/javase/9/docs/api/java/lang/StackWalker.html
+ final StackTraceElement[] callStack = Thread.currentThread().getStackTrace();
+
+ for (StackTraceElement stackFrame : callStack) {
+ boolean foundMatch = false;
+ for (String packageName : ignoredJavaPackages) {
+ final String classMethod = stackFrame.getClassName() + "." + stackFrame.getMethodName();
+ if (classMethod.startsWith(packageName)) {
+ foundMatch = true;
+ continue;
+ }
+ }
+ if (!foundMatch) {
+ return stackFrame.getClassName() + "." + stackFrame.getMethodName();
+ }
+ }
+ return "";
+ }
+
+
+
+ private static boolean isOakInternalQuery (String statement) {
+ return statement.endsWith(OAK_INTERNAL_MARKER);
Review comment:
that one would raise a NPE if the statement were to be null.... and without the annotation the default would be that it actually can be null..... but the check for null can be omitted if you make the contract clear... you never want a null query statement.
##########
File path: oak-core/src/test/java/org/apache/jackrabbit/oak/query/QueryInformationLoggerTest.java
##########
@@ -0,0 +1,143 @@
+package org.apache.jackrabbit.oak.query;
+
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.jackrabbit.oak.commons.junit.LogCustomizer;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.event.Level;
+
+public class QueryInformationLoggerTest {
+
+ private static final Logger LOG = LoggerFactory.getLogger(QueryInformationLoggerTest.class);
Review comment:
i might be mistaken.... but i haven't seen this LOG field used.... if it's not -> drop.
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryInformationLogger.java
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law
+ * or agreed to in writing, software distributed under the License is
+ * distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the specific language
+ * governing permissions and limitations under the License.
+ */
+package org.apache.jackrabbit.oak.query;
+
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class QueryInformationLogger {
+
+ private static final Logger INTERNAL_QUERIES_LOG = LoggerFactory.getLogger(QueryInformationLogger.class.getName() + ".internalQuery");
+ private static final Logger EXTERNAL_QUERIES_LOG = LoggerFactory.getLogger(QueryInformationLogger.class.getName() + ".externalQuery");
+
+ protected static final String OAK_INTERNAL_MARKER = "/* oak-internal */";
+
+ /**
+ * Logs the caller of the query based on the callstack. To refine the result, all stack frames
+ * are ignored, for which the entry (consisting of package name, classname and method name) starts
+ * with one of the entries provided by ignoredClassNames.
+ *
+ * @param statement the query statement
+ * @param ignoredClassNames entries to be ignored. If empty, the method is a no-op.
+ */
+ public static void logCaller (String statement, String[] ignoredClassNames) {
+
+ if (ignoredClassNames.length == 0) {
+ return;
+ }
+
+ if (isOakInternalQuery(statement)) {
+ INTERNAL_QUERIES_LOG.debug("Oak-internal query, query=[{}]", statement);
+ return;
+ }
+
+ String callingClass = getInvokingClass (ignoredClassNames);
+ if (callingClass.isEmpty()) {
+ INTERNAL_QUERIES_LOG.debug("Oak-internal query, query=[{}]", statement);
+ if (INTERNAL_QUERIES_LOG.isTraceEnabled()) {
+ try {
+ throw new RuntimeException("requested stacktrace");
+ } catch (RuntimeException e) {
+ INTERNAL_QUERIES_LOG.trace("providing requested stacktrace", e);
+ }
+ }
+ } else {
+ EXTERNAL_QUERIES_LOG.info("query=[{}], caller=[{}]",statement, callingClass);
+ if (EXTERNAL_QUERIES_LOG.isDebugEnabled()) {
+ try {
+ throw new RuntimeException("requested stacktrace");
+ } catch (RuntimeException e) {
+ EXTERNAL_QUERIES_LOG.debug("providing requested stacktrace", e);
+ }
+ }
+ }
+ }
+
+ /**
+ * Retrieves the calling class and method from the call stack; this is determined by unwinding
+ * the stack until it finds a combination of full qualified classname + method (separated by ".") which
+ * do not start with any of the values provided by the ignoredJavaPackages parameters.
+ *
+ * @param ignoredJavaPackages
+ * @return the calling class; if all classes of the call trace are in packages which are ignored,
+ * an empty string is returned.
+ */
+ private static String getInvokingClass (String[] ignoredJavaPackages) {
+
+ // With java9 we would use https://docs.oracle.com/javase/9/docs/api/java/lang/StackWalker.html
+ final StackTraceElement[] callStack = Thread.currentThread().getStackTrace();
+
+ for (StackTraceElement stackFrame : callStack) {
+ boolean foundMatch = false;
+ for (String packageName : ignoredJavaPackages) {
+ final String classMethod = stackFrame.getClassName() + "." + stackFrame.getMethodName();
+ if (classMethod.startsWith(packageName)) {
+ foundMatch = true;
+ continue;
+ }
+ }
+ if (!foundMatch) {
+ return stackFrame.getClassName() + "." + stackFrame.getMethodName();
+ }
+ }
+ return "";
+ }
+
+
+
+ private static boolean isOakInternalQuery (String statement) {
Review comment:
add @notnull annotation
##########
File path: oak-core/src/test/java/org/apache/jackrabbit/oak/query/QueryInformationLoggerTest.java
##########
@@ -0,0 +1,143 @@
+package org.apache.jackrabbit.oak.query;
+
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.jackrabbit.oak.commons.junit.LogCustomizer;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.event.Level;
+
+public class QueryInformationLoggerTest {
+
+ private static final Logger LOG = LoggerFactory.getLogger(QueryInformationLoggerTest.class);
+
+ private static final String EXTERNAL_LOG = QueryInformationLogger.class.getName() + ".externalQuery";
+ private static final String INTERNAL_LOG = QueryInformationLogger.class.getName() + ".internalQuery";
+
+ String[] ignoredClasses = new String[] {"org.apache.jackrabbit.oak","java.lang","sun.reflect", "jdk"};
+ String[] allClassesIgnored = new String[] {"java","org","net","sun","jdk"};
+
+
+ @Test
+ public void regularQueryTest() {
+
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.INFO).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+ assertEquals("Exactly 1 entry must be written",1,external.getLogs().size());
+ String logEntry = external.getLogs().get(0);
+ assertTrue(logEntry.contains("org.junit.runners.model")); // org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void withStacktrace() {
+
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+ assertEquals("Exactly 2 entries must be written",2,external.getLogs().size()); //
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void testOakInternalStatement() {
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.INFO).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page] " + QueryInformationLogger.OAK_INTERNAL_MARKER;
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+
+ // On INFO no logs are written
+ assertEquals(0,external.getLogs().size()); //
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void testWithAllStackEntriesIgnored_INFO() {
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.INFO).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, allClassesIgnored);
+ try {
+ throw new AssertionError();
+ } catch (AssertionError e) {
+ e.printStackTrace();
+ }
+
+ assertEquals(0,external.getLogs().size());
+ assertEquals(0,internal.getLogs().size()); // would require DEBUG
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+
+ @Test
+ public void testWithAllStackEntriesIgnored_DEBUG() {
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.DEBUG).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, allClassesIgnored);
+
+ assertEquals(0,external.getLogs().size());
+ assertEquals(1,internal.getLogs().size()); // query statement
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void testWithAllStackEntriesIgnored_TRACE() {
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, allClassesIgnored);
+
+ // On INFO no logs are written
+ assertEquals(0,external.getLogs().size());
+ assertEquals(2,internal.getLogs().size()); // query statement + Stacktrace
Review comment:
formattign again :)
##########
File path: oak-core/src/test/java/org/apache/jackrabbit/oak/query/QueryInformationLoggerTest.java
##########
@@ -0,0 +1,143 @@
+package org.apache.jackrabbit.oak.query;
+
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.jackrabbit.oak.commons.junit.LogCustomizer;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.event.Level;
+
+public class QueryInformationLoggerTest {
+
+ private static final Logger LOG = LoggerFactory.getLogger(QueryInformationLoggerTest.class);
+
+ private static final String EXTERNAL_LOG = QueryInformationLogger.class.getName() + ".externalQuery";
+ private static final String INTERNAL_LOG = QueryInformationLogger.class.getName() + ".internalQuery";
+
+ String[] ignoredClasses = new String[] {"org.apache.jackrabbit.oak","java.lang","sun.reflect", "jdk"};
+ String[] allClassesIgnored = new String[] {"java","org","net","sun","jdk"};
+
+
+ @Test
+ public void regularQueryTest() {
+
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.INFO).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+ assertEquals("Exactly 1 entry must be written",1,external.getLogs().size());
+ String logEntry = external.getLogs().get(0);
+ assertTrue(logEntry.contains("org.junit.runners.model")); // org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void withStacktrace() {
+
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+ assertEquals("Exactly 2 entries must be written",2,external.getLogs().size()); //
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void testOakInternalStatement() {
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.INFO).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page] " + QueryInformationLogger.OAK_INTERNAL_MARKER;
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+
+ // On INFO no logs are written
+ assertEquals(0,external.getLogs().size()); //
+ assertEquals(0,internal.getLogs().size());
Review comment:
formatting: add a space after ,
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryInformationLogger.java
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law
+ * or agreed to in writing, software distributed under the License is
+ * distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the specific language
+ * governing permissions and limitations under the License.
+ */
+package org.apache.jackrabbit.oak.query;
+
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class QueryInformationLogger {
+
+ private static final Logger INTERNAL_QUERIES_LOG = LoggerFactory.getLogger(QueryInformationLogger.class.getName() + ".internalQuery");
+ private static final Logger EXTERNAL_QUERIES_LOG = LoggerFactory.getLogger(QueryInformationLogger.class.getName() + ".externalQuery");
+
+ protected static final String OAK_INTERNAL_MARKER = "/* oak-internal */";
+
+ /**
+ * Logs the caller of the query based on the callstack. To refine the result, all stack frames
+ * are ignored, for which the entry (consisting of package name, classname and method name) starts
+ * with one of the entries provided by ignoredClassNames.
+ *
+ * @param statement the query statement
+ * @param ignoredClassNames entries to be ignored. If empty, the method is a no-op.
+ */
+ public static void logCaller (String statement, String[] ignoredClassNames) {
+
+ if (ignoredClassNames.length == 0) {
+ return;
+ }
+
+ if (isOakInternalQuery(statement)) {
+ INTERNAL_QUERIES_LOG.debug("Oak-internal query, query=[{}]", statement);
+ return;
+ }
+
+ String callingClass = getInvokingClass (ignoredClassNames);
+ if (callingClass.isEmpty()) {
+ INTERNAL_QUERIES_LOG.debug("Oak-internal query, query=[{}]", statement);
+ if (INTERNAL_QUERIES_LOG.isTraceEnabled()) {
+ try {
+ throw new RuntimeException("requested stacktrace");
+ } catch (RuntimeException e) {
+ INTERNAL_QUERIES_LOG.trace("providing requested stacktrace", e);
+ }
+ }
+ } else {
+ EXTERNAL_QUERIES_LOG.info("query=[{}], caller=[{}]",statement, callingClass);
+ if (EXTERNAL_QUERIES_LOG.isDebugEnabled()) {
+ try {
+ throw new RuntimeException("requested stacktrace");
+ } catch (RuntimeException e) {
+ EXTERNAL_QUERIES_LOG.debug("providing requested stacktrace", e);
+ }
+ }
+ }
+ }
+
+ /**
+ * Retrieves the calling class and method from the call stack; this is determined by unwinding
+ * the stack until it finds a combination of full qualified classname + method (separated by ".") which
+ * do not start with any of the values provided by the ignoredJavaPackages parameters.
+ *
+ * @param ignoredJavaPackages
Review comment:
javadoc description is missing..... but ok.... it's a private class and it's obvious from the context....
so really minor nitpicking :)
##########
File path: oak-core/src/test/java/org/apache/jackrabbit/oak/query/QueryInformationLoggerTest.java
##########
@@ -0,0 +1,143 @@
+package org.apache.jackrabbit.oak.query;
+
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.jackrabbit.oak.commons.junit.LogCustomizer;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.event.Level;
+
+public class QueryInformationLoggerTest {
+
+ private static final Logger LOG = LoggerFactory.getLogger(QueryInformationLoggerTest.class);
+
+ private static final String EXTERNAL_LOG = QueryInformationLogger.class.getName() + ".externalQuery";
+ private static final String INTERNAL_LOG = QueryInformationLogger.class.getName() + ".internalQuery";
+
+ String[] ignoredClasses = new String[] {"org.apache.jackrabbit.oak","java.lang","sun.reflect", "jdk"};
+ String[] allClassesIgnored = new String[] {"java","org","net","sun","jdk"};
+
+
+ @Test
+ public void regularQueryTest() {
+
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.INFO).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+ assertEquals("Exactly 1 entry must be written",1,external.getLogs().size());
+ String logEntry = external.getLogs().get(0);
+ assertTrue(logEntry.contains("org.junit.runners.model")); // org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void withStacktrace() {
+
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+ assertEquals("Exactly 2 entries must be written",2,external.getLogs().size()); //
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void testOakInternalStatement() {
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.INFO).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page] " + QueryInformationLogger.OAK_INTERNAL_MARKER;
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+
+ // On INFO no logs are written
+ assertEquals(0,external.getLogs().size()); //
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void testWithAllStackEntriesIgnored_INFO() {
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.INFO).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, allClassesIgnored);
+ try {
+ throw new AssertionError();
+ } catch (AssertionError e) {
+ e.printStackTrace();
+ }
+
+ assertEquals(0,external.getLogs().size());
+ assertEquals(0,internal.getLogs().size()); // would require DEBUG
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+
+ @Test
+ public void testWithAllStackEntriesIgnored_DEBUG() {
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.DEBUG).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, allClassesIgnored);
+
+ assertEquals(0,external.getLogs().size());
+ assertEquals(1,internal.getLogs().size()); // query statement
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void testWithAllStackEntriesIgnored_TRACE() {
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, allClassesIgnored);
+
+ // On INFO no logs are written
+ assertEquals(0,external.getLogs().size());
Review comment:
- formatting: missing space after ,
- potential simplification see above
##########
File path: oak-core/src/test/java/org/apache/jackrabbit/oak/query/QueryInformationLoggerTest.java
##########
@@ -0,0 +1,143 @@
+package org.apache.jackrabbit.oak.query;
+
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.jackrabbit.oak.commons.junit.LogCustomizer;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.event.Level;
+
+public class QueryInformationLoggerTest {
+
+ private static final Logger LOG = LoggerFactory.getLogger(QueryInformationLoggerTest.class);
+
+ private static final String EXTERNAL_LOG = QueryInformationLogger.class.getName() + ".externalQuery";
+ private static final String INTERNAL_LOG = QueryInformationLogger.class.getName() + ".internalQuery";
+
+ String[] ignoredClasses = new String[] {"org.apache.jackrabbit.oak","java.lang","sun.reflect", "jdk"};
+ String[] allClassesIgnored = new String[] {"java","org","net","sun","jdk"};
+
+
+ @Test
+ public void regularQueryTest() {
+
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.INFO).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+ assertEquals("Exactly 1 entry must be written",1,external.getLogs().size());
+ String logEntry = external.getLogs().get(0);
+ assertTrue(logEntry.contains("org.junit.runners.model")); // org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void withStacktrace() {
+
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+ assertEquals("Exactly 2 entries must be written",2,external.getLogs().size()); //
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void testOakInternalStatement() {
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.INFO).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page] " + QueryInformationLogger.OAK_INTERNAL_MARKER;
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+
+ // On INFO no logs are written
+ assertEquals(0,external.getLogs().size()); //
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void testWithAllStackEntriesIgnored_INFO() {
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.INFO).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, allClassesIgnored);
+ try {
+ throw new AssertionError();
+ } catch (AssertionError e) {
+ e.printStackTrace();
+ }
+
+ assertEquals(0,external.getLogs().size());
+ assertEquals(0,internal.getLogs().size()); // would require DEBUG
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+
+ @Test
+ public void testWithAllStackEntriesIgnored_DEBUG() {
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.DEBUG).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, allClassesIgnored);
+
+ assertEquals(0,external.getLogs().size());
+ assertEquals(1,internal.getLogs().size()); // query statement
Review comment:
see above.... formatting
##########
File path: oak-core/src/test/java/org/apache/jackrabbit/oak/query/QueryInformationLoggerTest.java
##########
@@ -0,0 +1,143 @@
+package org.apache.jackrabbit.oak.query;
+
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.jackrabbit.oak.commons.junit.LogCustomizer;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.event.Level;
+
+public class QueryInformationLoggerTest {
+
+ private static final Logger LOG = LoggerFactory.getLogger(QueryInformationLoggerTest.class);
+
+ private static final String EXTERNAL_LOG = QueryInformationLogger.class.getName() + ".externalQuery";
+ private static final String INTERNAL_LOG = QueryInformationLogger.class.getName() + ".internalQuery";
+
+ String[] ignoredClasses = new String[] {"org.apache.jackrabbit.oak","java.lang","sun.reflect", "jdk"};
+ String[] allClassesIgnored = new String[] {"java","org","net","sun","jdk"};
+
+
+ @Test
+ public void regularQueryTest() {
+
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.INFO).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+ assertEquals("Exactly 1 entry must be written",1,external.getLogs().size());
+ String logEntry = external.getLogs().get(0);
+ assertTrue(logEntry.contains("org.junit.runners.model")); // org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void withStacktrace() {
+
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+ assertEquals("Exactly 2 entries must be written",2,external.getLogs().size()); //
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void testOakInternalStatement() {
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.INFO).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page] " + QueryInformationLogger.OAK_INTERNAL_MARKER;
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+
+ // On INFO no logs are written
+ assertEquals(0,external.getLogs().size()); //
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void testWithAllStackEntriesIgnored_INFO() {
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.INFO).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, allClassesIgnored);
+ try {
+ throw new AssertionError();
+ } catch (AssertionError e) {
+ e.printStackTrace();
+ }
+
+ assertEquals(0,external.getLogs().size());
Review comment:
formatting again :)
you could simplify as `assertTrue(external.getLogs().isEmpty()`.. but that's probably a matter of taste.
##########
File path: oak-core/src/test/java/org/apache/jackrabbit/oak/query/QueryInformationLoggerTest.java
##########
@@ -0,0 +1,143 @@
+package org.apache.jackrabbit.oak.query;
+
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.jackrabbit.oak.commons.junit.LogCustomizer;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.event.Level;
+
+public class QueryInformationLoggerTest {
+
+ private static final Logger LOG = LoggerFactory.getLogger(QueryInformationLoggerTest.class);
+
+ private static final String EXTERNAL_LOG = QueryInformationLogger.class.getName() + ".externalQuery";
+ private static final String INTERNAL_LOG = QueryInformationLogger.class.getName() + ".internalQuery";
+
+ String[] ignoredClasses = new String[] {"org.apache.jackrabbit.oak","java.lang","sun.reflect", "jdk"};
+ String[] allClassesIgnored = new String[] {"java","org","net","sun","jdk"};
+
+
+ @Test
+ public void regularQueryTest() {
+
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.INFO).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+ assertEquals("Exactly 1 entry must be written",1,external.getLogs().size());
+ String logEntry = external.getLogs().get(0);
+ assertTrue(logEntry.contains("org.junit.runners.model")); // org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void withStacktrace() {
+
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+ assertEquals("Exactly 2 entries must be written",2,external.getLogs().size()); //
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void testOakInternalStatement() {
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.INFO).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page] " + QueryInformationLogger.OAK_INTERNAL_MARKER;
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+
+ // On INFO no logs are written
+ assertEquals(0,external.getLogs().size()); //
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void testWithAllStackEntriesIgnored_INFO() {
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.INFO).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, allClassesIgnored);
+ try {
+ throw new AssertionError();
+ } catch (AssertionError e) {
+ e.printStackTrace();
+ }
+
+ assertEquals(0,external.getLogs().size());
+ assertEquals(0,internal.getLogs().size()); // would require DEBUG
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+
+ @Test
+ public void testWithAllStackEntriesIgnored_DEBUG() {
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.DEBUG).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, allClassesIgnored);
+
+ assertEquals(0,external.getLogs().size());
Review comment:
see above. formatting and potential simplification.
##########
File path: oak-core/src/test/java/org/apache/jackrabbit/oak/query/QueryInformationLoggerTest.java
##########
@@ -0,0 +1,143 @@
+package org.apache.jackrabbit.oak.query;
+
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.jackrabbit.oak.commons.junit.LogCustomizer;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.event.Level;
+
+public class QueryInformationLoggerTest {
+
+ private static final Logger LOG = LoggerFactory.getLogger(QueryInformationLoggerTest.class);
+
+ private static final String EXTERNAL_LOG = QueryInformationLogger.class.getName() + ".externalQuery";
+ private static final String INTERNAL_LOG = QueryInformationLogger.class.getName() + ".internalQuery";
+
+ String[] ignoredClasses = new String[] {"org.apache.jackrabbit.oak","java.lang","sun.reflect", "jdk"};
+ String[] allClassesIgnored = new String[] {"java","org","net","sun","jdk"};
+
+
+ @Test
+ public void regularQueryTest() {
+
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.INFO).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+ assertEquals("Exactly 1 entry must be written",1,external.getLogs().size());
+ String logEntry = external.getLogs().get(0);
+ assertTrue(logEntry.contains("org.junit.runners.model")); // org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
+ assertEquals(0,internal.getLogs().size());
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+ @Test
+ public void withStacktrace() {
+
+ LogCustomizer external = LogCustomizer.forLogger(EXTERNAL_LOG).enable(Level.DEBUG).create();
+ LogCustomizer internal = LogCustomizer.forLogger(INTERNAL_LOG).enable(Level.TRACE).create();
+ try {
+ external.starting();
+ internal.starting();
+ String query = "SELECT * FROM [cq:Page]";
+ QueryInformationLogger.logCaller(query, ignoredClasses);
+ assertEquals("Exactly 2 entries must be written",2,external.getLogs().size()); //
Review comment:
formatting: add a space after the ,
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org