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/17 18:21:09 UTC
[GitHub] [jackrabbit-oak] joerghoh opened a new pull request #419: OAK-9624 add query caller logging
joerghoh opened a new pull request #419:
URL: https://github.com/apache/jackrabbit-oak/pull/419
* the configuration of an ignoreList is done via the OSGI configuration (QueryEngineSettingsService)
* if the ignoreList is empty (non-configured), nothing is done, and there is no overhead.
* If configured it displays the caller based on the callstack, ignoring stackframes which start with the classnames on the ignoreList.
* If a more verbose loglevel is used, also full stacktraces are logged (helps to refine the configuration)
* Dedicated loggers are created for internal queries (which either are completely matched by elements of the ignoreList or which have the ``/* oak-internal */``a nnotation) or external queries (everything else).
* For external queries a single log line is logged on INFO (stacktrace on DEBUG), for internal queries the single log line is logged on DEBUG (stacktrace on TRACE).
There might be a small overhead in execution time (getting the stack isn't that cheap), but for the frequent and time-critical queries (the ones marked with ``/* oak-internal */`` this isn't done.
When these elements are put onto the ignoreList:
```
org.apache.jackrabbit.oak
java.lang
org.apache.sling.jcr.resource.internal.helper.JcrResourceUtil.query
org.apache.sling.jcr.resource.internal.helper.jcr.BasicQueryLanguageProvider
org.apache.sling.resourceresolver.impl.providers.stateful.AuthenticatedResourceProvider.findResources
org.apache.sling.resourceresolver.impl.helper.ResourceResolverControl.findResources
org.apache.sling.resourceresolver.impl.ResourceResolverImpl.findResources
```
The correct calling classes of JCR Queries and Sling's ``ResourceResolver.findResource`` are logged.
--
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
[GitHub] [jackrabbit-oak] joerghoh commented on a change in pull request #419: OAK-9624 add query caller logging
Posted by GitBox <gi...@apache.org>.
joerghoh commented on a change in pull request #419:
URL: https://github.com/apache/jackrabbit-oak/pull/419#discussion_r752203085
##########
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:
Fixed
##########
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:
Fixed
--
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
[GitHub] [jackrabbit-oak] joerghoh commented on a change in pull request #419: OAK-9624 add query caller logging
Posted by GitBox <gi...@apache.org>.
joerghoh commented on a change in pull request #419:
URL: https://github.com/apache/jackrabbit-oak/pull/419#discussion_r752188780
##########
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:
I have never seen this annotation being used in a class annotated with ```@interface```, as this is normally not instantiated directly, but just created by some OSGI magic. Also the default value makes sure that it is never null.
--
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
[GitHub] [jackrabbit-oak] jsedding commented on a change in pull request #419: OAK-9624 add query caller logging
Posted by GitBox <gi...@apache.org>.
jsedding commented on a change in pull request #419:
URL: https://github.com/apache/jackrabbit-oak/pull/419#discussion_r753121478
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryInformationLogger.java
##########
@@ -0,0 +1,111 @@
+/*
+ * 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.apache.jackrabbit.oak.api.QueryEngine;
+import org.jetbrains.annotations.NotNull;
+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");
+
+ private QueryInformationLogger() {
+ // prevent instantiation
+ }
+
+ /**
+ * 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(@NotNull String statement, @NotNull 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()) {
+ // If all stackframes are covered by the ignoredClassNames, it's something which is
+ // explicitly wanted not to be logged along other queries for which this is not true.
+ 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.
+ */
+ @NotNull
+ private static String getInvokingClass(@NotNull 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();
+ }
+ }
Review comment:
The logic in this loop seems a bit hard to read and could be more efficient (it calls `stackFrame.getClassName()` only once per `stackFrame` and `pkg`).
```suggestion
for (StackTraceElement stackFrame : callStack) {
final String className = stackFrame.getClassName();
if (Stream.of(ignoredJavaPackages).noneMatch(pkg -> className.startsWith(pkg))) {
return className + "." + stackFrame.getMethodName();
}
}
```
--
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
[GitHub] [jackrabbit-oak] joerghoh commented on a change in pull request #419: OAK-9624 add query caller logging
Posted by GitBox <gi...@apache.org>.
joerghoh commented on a change in pull request #419:
URL: https://github.com/apache/jackrabbit-oak/pull/419#discussion_r752193767
##########
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:
Good point, ```QueryEngine.INTERNAL_SQL2_QUERY``` is it.
--
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
[GitHub] [jackrabbit-oak] joerghoh closed pull request #419: OAK-9624 add query caller logging
Posted by GitBox <gi...@apache.org>.
joerghoh closed pull request #419:
URL: https://github.com/apache/jackrabbit-oak/pull/419
--
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
[GitHub] [jackrabbit-oak] joerghoh commented on a change in pull request #419: OAK-9624 add query caller logging
Posted by GitBox <gi...@apache.org>.
joerghoh commented on a change in pull request #419:
URL: https://github.com/apache/jackrabbit-oak/pull/419#discussion_r753829352
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryInformationLogger.java
##########
@@ -0,0 +1,111 @@
+/*
+ * 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.apache.jackrabbit.oak.api.QueryEngine;
+import org.jetbrains.annotations.NotNull;
+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");
+
+ private QueryInformationLogger() {
+ // prevent instantiation
+ }
+
+ /**
+ * 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(@NotNull String statement, @NotNull 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()) {
+ // If all stackframes are covered by the ignoredClassNames, it's something which is
+ // explicitly wanted not to be logged along other queries for which this is not true.
+ 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.
+ */
+ @NotNull
+ private static String getInvokingClass(@NotNull 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();
+ }
+ }
Review comment:
Thanks for the suggestion, I will slightly adapt it and add the ability to ignore specific method calls as well.
Regarding the implementation: We want to explicitly be able to find the creators of _all_ queries, not just the slow ones.
--
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
[GitHub] [jackrabbit-oak] joerghoh commented on a change in pull request #419: OAK-9624 add query caller logging
Posted by GitBox <gi...@apache.org>.
joerghoh commented on a change in pull request #419:
URL: https://github.com/apache/jackrabbit-oak/pull/419#discussion_r753829352
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryInformationLogger.java
##########
@@ -0,0 +1,111 @@
+/*
+ * 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.apache.jackrabbit.oak.api.QueryEngine;
+import org.jetbrains.annotations.NotNull;
+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");
+
+ private QueryInformationLogger() {
+ // prevent instantiation
+ }
+
+ /**
+ * 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(@NotNull String statement, @NotNull 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()) {
+ // If all stackframes are covered by the ignoredClassNames, it's something which is
+ // explicitly wanted not to be logged along other queries for which this is not true.
+ 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.
+ */
+ @NotNull
+ private static String getInvokingClass(@NotNull 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();
+ }
+ }
Review comment:
Thanks for the suggestion, I will slightly adapt it and add the ability to ignore specific method calls as well.
Regarding the implementation: We want to explicitly be able to find the creators of _all_ queries, not just the slow ones.
--
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
[GitHub] [jackrabbit-oak] anchela commented on a change in pull request #419: OAK-9624 add query caller logging
Posted by GitBox <gi...@apache.org>.
anchela commented on a change in pull request #419:
URL: https://github.com/apache/jackrabbit-oak/pull/419#discussion_r752241839
##########
File path: oak-api/src/main/java/org/apache/jackrabbit/oak/api/jmx/QueryEngineSettingsMBean.java
##########
@@ -139,5 +140,23 @@ void setQueryValidatorPattern(
@Description("Get the query validator data as a JSON string.")
String getQueryValidatorJson();
-
+
+
+ /**
+ * Set or remove java package/class names which are ignored from finding the
+ * invoking class for queries.
+ *
+ * it can be either Java package names or fully-qualified class names (package + class name).
+ *
+ * @param classNames the class names to be ignored.
+ */
+
+ @Description("Set or remove Java Package Name to ignore in Call Trace analysis")
+ void setIgnoredClassNamesinCallTrace(
+ @Description("package or fully qualified class names")
+ @Name("class names")
+ @NotNull String[] classNames);
+
+ @Description("Get the Java package / fully qualified class names to ignore when finding the caller of query")
+ @NotNull String[] getIgnoredClassNamesinCallTrace();
Review comment:
hi @joerghoh i looked at your patch again in my IDE not just the diff on github. and my IDE complains about a typo in this method name. it should be `getIgnoredClassNamesInCallTrace`
##########
File path: oak-api/src/main/java/org/apache/jackrabbit/oak/api/jmx/QueryEngineSettingsMBean.java
##########
@@ -139,5 +140,23 @@ void setQueryValidatorPattern(
@Description("Get the query validator data as a JSON string.")
String getQueryValidatorJson();
-
+
+
+ /**
+ * Set or remove java package/class names which are ignored from finding the
+ * invoking class for queries.
+ *
+ * it can be either Java package names or fully-qualified class names (package + class name).
+ *
+ * @param classNames the class names to be ignored.
+ */
+
+ @Description("Set or remove Java Package Name to ignore in Call Trace analysis")
+ void setIgnoredClassNamesinCallTrace(
Review comment:
my IDE complains about a typo in the method name: should be `setIgnoredClassNamesInCallTrace`
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryInformationLogger.java
##########
@@ -0,0 +1,120 @@
+/*
+ * 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.apache.jackrabbit.oak.api.QueryEngine;
+import org.jetbrains.annotations.NotNull;
+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");
+
+ private QueryInformationLogger() {
+ // prevent instantiation
+ }
+
+ /**
+ * 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(@NotNull String statement, @NotNull 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()) {
+ // If all stackframes are covered by the ignoredClassNames, it's something which is
+ // explicitly wanted not to be logged along other queries for which this is not true.
+ 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 "";
+ }
+
+
+ /**
+ * Check if statement is explicitly marked to be internal
+ * @param statement the JCR query statement to check
+ * @return true the query is marked to be an internal query
+ */
+ private static boolean isOakInternalQuery(@NotNull String statement) {
+ return statement.endsWith(QueryEngine.INTERNAL_SQL2_QUERY);
+ }
+
Review comment:
extra lines until end of class -> drop
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/Oak.java
##########
@@ -974,6 +974,15 @@ public String toString() {
void setFullTextComparisonWithoutIndex(boolean fullTextComparisonWithoutIndex) {
this.settings.setFullTextComparisonWithoutIndex(fullTextComparisonWithoutIndex);
}
+
+ public void setIgnoredClassNamesinCallTrace(@NotNull String[] packageNames) {
Review comment:
same here.... `setIgnoredClassNamesInCallTrace`
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/Oak.java
##########
@@ -974,6 +974,15 @@ public String toString() {
void setFullTextComparisonWithoutIndex(boolean fullTextComparisonWithoutIndex) {
this.settings.setFullTextComparisonWithoutIndex(fullTextComparisonWithoutIndex);
}
+
+ public void setIgnoredClassNamesinCallTrace(@NotNull String[] packageNames) {
+ settings.setIgnoredClassNamesinCallTrace(packageNames);
+ }
+
+ public @NotNull String[] getIgnoredClassNamesinCallTrace() {
Review comment:
and here `getIgnoredClassNamesInCallTrace`
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryInformationLogger.java
##########
@@ -0,0 +1,120 @@
+/*
+ * 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.apache.jackrabbit.oak.api.QueryEngine;
+import org.jetbrains.annotations.NotNull;
+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");
+
+ private QueryInformationLogger() {
+ // prevent instantiation
+ }
+
+ /**
+ * 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(@NotNull String statement, @NotNull 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()) {
+ // If all stackframes are covered by the ignoredClassNames, it's something which is
+ // explicitly wanted not to be logged along other queries for which this is not true.
+ INTERNAL_QUERIES_LOG.debug("Oak-internal query, query=[{}]", statement);
+ if (INTERNAL_QUERIES_LOG.isTraceEnabled()) {
+ try {
Review comment:
this try-catch looks a bit odd.... i wonder if it could be replaced by just writing:
`INTERNAL_QUERIES_LOG.trace("providing requested stacktrace", new RuntimeException("requested stacktrace");`
but i am not 100% sure....
##########
File path: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryInformationLogger.java
##########
@@ -0,0 +1,120 @@
+/*
+ * 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.apache.jackrabbit.oak.api.QueryEngine;
+import org.jetbrains.annotations.NotNull;
+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");
+
+ private QueryInformationLogger() {
+ // prevent instantiation
+ }
+
+ /**
+ * 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(@NotNull String statement, @NotNull 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()) {
+ // If all stackframes are covered by the ignoredClassNames, it's something which is
+ // explicitly wanted not to be logged along other queries for which this is not true.
+ 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 {
Review comment:
same here
##########
File path: oak-core/src/test/java/org/apache/jackrabbit/oak/query/QueryInformationLoggerTest.java
##########
@@ -0,0 +1,141 @@
+package org.apache.jackrabbit.oak.query;
+
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.jackrabbit.oak.api.QueryEngine;
+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 String EXTERNAL_LOG = QueryInformationLogger.class.getName() + ".externalQuery";
+ private static final String INTERNAL_LOG = QueryInformationLogger.class.getName() + ".internalQuery";
+
+ // these package names should ignore all Oak-internal code
+ String[] ignoredClasses = new String[] {"org.apache.jackrabbit.oak","java.lang","sun.reflect", "jdk"};
+
+ // these package names should cover all class names which appear in the stack trace.
+ // it might be required to adjust if the CI/CD solution uses different package names (e.g. "com")
+ 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] " + QueryEngine.INTERNAL_SQL2_QUERY;
+ 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);
+
+ assertEquals(0, external.getLogs().size());
+ assertEquals(0, internal.getLogs().size()); // would require DEBUG
+ } finally {
+ external.finished();
+ internal.finished();
+ }
+ }
+
+
Review comment:
remove 1 empty line
##########
File path: oak-core/src/test/java/org/apache/jackrabbit/oak/query/QueryInformationLoggerTest.java
##########
@@ -0,0 +1,141 @@
+package org.apache.jackrabbit.oak.query;
+
Review comment:
one line too much :)
##########
File path: oak-core/src/test/java/org/apache/jackrabbit/oak/query/QueryInformationLoggerTest.java
##########
@@ -0,0 +1,141 @@
+package org.apache.jackrabbit.oak.query;
Review comment:
missing license header.....
--
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
[GitHub] [jackrabbit-oak] joerghoh commented on a change in pull request #419: OAK-9624 add query caller logging
Posted by GitBox <gi...@apache.org>.
joerghoh commented on a change in pull request #419:
URL: https://github.com/apache/jackrabbit-oak/pull/419#discussion_r752203375
##########
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:
removed
--
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
[GitHub] [jackrabbit-oak] joerghoh commented on pull request #419: OAK-9624 add query caller logging
Posted by GitBox <gi...@apache.org>.
joerghoh commented on pull request #419:
URL: https://github.com/apache/jackrabbit-oak/pull/419#issuecomment-975353695
I redefined the logging strategy with @thomasmueller and this approach does not seem to be too useful. I will close this PR and come up with a new one.
Thanks to @anchela and @jsedding for your help, I will incorporate your feedback into the next iteration of this feature.
--
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
[GitHub] [jackrabbit-oak] joerghoh commented on a change in pull request #419: OAK-9624 add query caller logging
Posted by GitBox <gi...@apache.org>.
joerghoh commented on a change in pull request #419:
URL: https://github.com/apache/jackrabbit-oak/pull/419#discussion_r752188780
##########
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:
I have never seen this annotation being used in a class annotated with ```@interface```, as this is normally not instantiated directly, but just created by some OSGI magic. Will add an explicit default value though.
--
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
[GitHub] [jackrabbit-oak] joerghoh commented on a change in pull request #419: OAK-9624 add query caller logging
Posted by GitBox <gi...@apache.org>.
joerghoh commented on a change in pull request #419:
URL: https://github.com/apache/jackrabbit-oak/pull/419#discussion_r752204067
##########
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:
added a private constructor
--
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
[GitHub] [jackrabbit-oak] anchela commented on a change in pull request #419: OAK-9624 add query caller logging
Posted by GitBox <gi...@apache.org>.
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