You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2022/01/14 16:26:05 UTC
[GitHub] [logging-log4j2] vorburger opened a new pull request #698: Support source class & method in to-jul (LOG4J2-3282)
vorburger opened a new pull request #698:
URL: https://github.com/apache/logging-log4j2/pull/698
This is a LOG4J2-3282 related follow-up to #653.
@carterkozak is this what you were meant [in our conversation here](https://github.com/apache/logging-log4j2/pull/653#discussion_r778374993)? (Sorry at the time I didn't get 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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] garydgregory commented on a change in pull request #698: Support source class & method in to-jul (LOG4J2-3282)
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #698:
URL: https://github.com/apache/logging-log4j2/pull/698#discussion_r785020235
##########
File path: log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/LazyLog4jLogRecord.java
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.logging.log4j.tojul;
+
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import org.apache.logging.log4j.util.StackLocatorUtil;
+
+/**
+ * Extension of {@link java.util.logging.LogRecord} with lazy get source related methods based on Log4j's {@link StackLocatorUtil#calcLocation(String)}.
+ *
+ * @author <a href="http://www.vorburger.ch">Michael Vorburger.ch</a> for Google
+ */
+/* package-local */ final class LazyLog4jLogRecord extends LogRecord {
+
+ private static final long serialVersionUID = 6798134264543826471L;
+
+ // parent class LogRecord already has a needToInferCaller but it's private
+ private transient boolean stillNeedToInferCaller = true;
Review comment:
At least stillNeedToInferCaller -> inferCaller or flip the boolean value and call it callerInitialized?
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] vorburger commented on a change in pull request #698: Support source class & method in to-jul (LOG4J2-3282)
Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #698:
URL: https://github.com/apache/logging-log4j2/pull/698#discussion_r785075733
##########
File path: log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/LazyLog4jLogRecord.java
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.logging.log4j.tojul;
+
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import org.apache.logging.log4j.util.StackLocatorUtil;
+
+/**
+ * Extension of {@link java.util.logging.LogRecord} with lazy get source related methods based on Log4j's {@link StackLocatorUtil#calcLocation(String)}.
+ */
+final class LazyLog4jLogRecord extends LogRecord {
+
+ private static final long serialVersionUID = 6798134264543826471L;
+
+ // parent class LogRecord already has a needToInferCaller but it's private
+ private transient boolean inferCaller = true;
+
+ private final String fqcn;
+
+ LazyLog4jLogRecord(String fqcn, Level level, String msg) {
+ super(level, msg);
+ this.fqcn = fqcn;
+ }
+
+ @Override
+ public String getSourceClassName() {
+ if (inferCaller) {
+ inferCaller();
+ }
+ return super.getSourceClassName();
+ }
+
+ @Override
+ public String getSourceMethodName() {
+ if (inferCaller) {
+ inferCaller();
+ }
+ return super.getSourceMethodName();
+ }
+
+ private synchronized void inferCaller() {
Review comment:
@garydgregory @carterkozak can I let you guys hash this out among ourselves and let me know your conclusion? I'm happy if you are (both) happy, but I'd prefer not going back and forth a few times between you... 😸 I believe this comment appears to request to (re-)remove `synchronized` (again, after @garydgregory suggested it be added), and also for `volatile` to be removed from `inferCaller`. Please both ACK?
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] garydgregory commented on a change in pull request #698: Support source class & method in to-jul (LOG4J2-3282)
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #698:
URL: https://github.com/apache/logging-log4j2/pull/698#discussion_r785020969
##########
File path: log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/LazyLog4jLogRecord.java
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.logging.log4j.tojul;
+
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import org.apache.logging.log4j.util.StackLocatorUtil;
+
+/**
+ * Extension of {@link java.util.logging.LogRecord} with lazy get source related methods based on Log4j's {@link StackLocatorUtil#calcLocation(String)}.
+ *
+ * @author <a href="http://www.vorburger.ch">Michael Vorburger.ch</a> for Google
+ */
+/* package-local */ final class LazyLog4jLogRecord extends LogRecord {
+
+ private static final long serialVersionUID = 6798134264543826471L;
+
+ // parent class LogRecord already has a needToInferCaller but it's private
+ private transient boolean stillNeedToInferCaller = true;
+
+ private final String fqcn;
+
+ LazyLog4jLogRecord(String fqcn, Level level, String msg) {
+ super(level, msg);
+ this.fqcn = fqcn;
+ }
+
+ @Override
+ public String getSourceClassName() {
+ if (stillNeedToInferCaller) {
+ inferCaller();
+ }
+ return super.getSourceClassName();
+ }
+
+ @Override
+ public String getSourceMethodName() {
+ if (stillNeedToInferCaller) {
+ inferCaller();
+ }
+ return super.getSourceMethodName();
+ }
+
+ private void inferCaller() {
Review comment:
Should this be synchronized to avoid extra work?
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak commented on a change in pull request #698: Support source class & method in to-jul (LOG4J2-3282)
Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #698:
URL: https://github.com/apache/logging-log4j2/pull/698#discussion_r785030436
##########
File path: log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/LazyLog4jLogRecord.java
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.logging.log4j.tojul;
+
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import org.apache.logging.log4j.util.StackLocatorUtil;
+
+/**
+ * Extension of {@link java.util.logging.LogRecord} with lazy get source related methods based on Log4j's {@link StackLocatorUtil#calcLocation(String)}.
+ *
+ * @author <a href="http://www.vorburger.ch">Michael Vorburger.ch</a> for Google
+ */
+/* package-local */ final class LazyLog4jLogRecord extends LogRecord {
+
+ private static final long serialVersionUID = 6798134264543826471L;
+
+ // parent class LogRecord already has a needToInferCaller but it's private
+ private transient boolean stillNeedToInferCaller = true;
+
+ private final String fqcn;
+
+ LazyLog4jLogRecord(String fqcn, Level level, String msg) {
+ super(level, msg);
+ this.fqcn = fqcn;
+ }
+
+ @Override
+ public String getSourceClassName() {
+ if (stillNeedToInferCaller) {
+ inferCaller();
+ }
+ return super.getSourceClassName();
+ }
+
+ @Override
+ public String getSourceMethodName() {
+ if (stillNeedToInferCaller) {
+ inferCaller();
+ }
+ return super.getSourceMethodName();
+ }
+
+ private void inferCaller() {
Review comment:
synchronized creates more work than it avoids! If a logrecord is passed to another thread it must either be wrapped to avoid searching for callsite data, or must be filled in prior to being passed (much like log4j createMemento)
This matches `java.util.logging.LogRecord` semantics.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak commented on a change in pull request #698: Support source class & method in to-jul (LOG4J2-3282)
Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #698:
URL: https://github.com/apache/logging-log4j2/pull/698#discussion_r785134421
##########
File path: log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/LazyLog4jLogRecord.java
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.logging.log4j.tojul;
+
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import org.apache.logging.log4j.util.StackLocatorUtil;
+
+/**
+ * Extension of {@link java.util.logging.LogRecord} with lazy get source related methods based on Log4j's {@link StackLocatorUtil#calcLocation(String)}.
+ */
+final class LazyLog4jLogRecord extends LogRecord {
+
+ private static final long serialVersionUID = 6798134264543826471L;
+
+ // parent class LogRecord already has a needToInferCaller but it's private
+ private transient boolean inferCaller = true;
+
+ private final String fqcn;
+
+ LazyLog4jLogRecord(String fqcn, Level level, String msg) {
+ super(level, msg);
+ this.fqcn = fqcn;
+ }
+
+ @Override
+ public String getSourceClassName() {
+ if (inferCaller) {
+ inferCaller();
+ }
+ return super.getSourceClassName();
+ }
+
+ @Override
+ public String getSourceMethodName() {
+ if (inferCaller) {
+ inferCaller();
+ }
+ return super.getSourceMethodName();
+ }
+
+ private synchronized void inferCaller() {
Review comment:
Let's remove the `synchronized` keyword please, sorry for the churn!
> and also for volatile to be removed from inferCaller
I don't think `inferCaller` is volatile, no change needed there!
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak commented on pull request #698: Support source class & method in to-jul (LOG4J2-3282)
Posted by GitBox <gi...@apache.org>.
carterkozak commented on pull request #698:
URL: https://github.com/apache/logging-log4j2/pull/698#issuecomment-1013429061
Thanks, Gary :-)
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak commented on a change in pull request #698: Support source class & method in to-jul (LOG4J2-3282)
Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #698:
URL: https://github.com/apache/logging-log4j2/pull/698#discussion_r785073240
##########
File path: log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/LazyLog4jLogRecord.java
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.logging.log4j.tojul;
+
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import org.apache.logging.log4j.util.StackLocatorUtil;
+
+/**
+ * Extension of {@link java.util.logging.LogRecord} with lazy get source related methods based on Log4j's {@link StackLocatorUtil#calcLocation(String)}.
+ */
+final class LazyLog4jLogRecord extends LogRecord {
+
+ private static final long serialVersionUID = 6798134264543826471L;
+
+ // parent class LogRecord already has a needToInferCaller but it's private
+ private transient boolean inferCaller = true;
+
+ private final String fqcn;
+
+ LazyLog4jLogRecord(String fqcn, Level level, String msg) {
+ super(level, msg);
+ this.fqcn = fqcn;
+ }
+
+ @Override
+ public String getSourceClassName() {
+ if (inferCaller) {
+ inferCaller();
+ }
+ return super.getSourceClassName();
+ }
+
+ @Override
+ public String getSourceMethodName() {
+ if (inferCaller) {
+ inferCaller();
+ }
+ return super.getSourceMethodName();
+ }
+
+ private synchronized void inferCaller() {
+ StackTraceElement location = StackLocatorUtil.calcLocation(fqcn);
+ setSourceClassName(location.getClassName());
+ setSourceMethodName(location.getMethodName());
Review comment:
I think we may need some null checks here in case `fqcn` is null, or the resulting `location` is null. In either case we should probably explicitly set the `sourceClassName` and `sourceMethodName` to `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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] vorburger commented on pull request #698: Support source class & method in to-jul (LOG4J2-3282)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #698:
URL: https://github.com/apache/logging-log4j2/pull/698#issuecomment-1013287207
> Just needs a license header to appease rat.
Duh! I should know better; done!
> We might consider naming the record type something more descriptive like `Log4jLogRecord`
Done, now `LazyLog4jLogRecord` as that's really what this is (IMHO).
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] garydgregory commented on a change in pull request #698: Support source class & method in to-jul (LOG4J2-3282)
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #698:
URL: https://github.com/apache/logging-log4j2/pull/698#discussion_r785019107
##########
File path: log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/LazyLog4jLogRecord.java
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.logging.log4j.tojul;
+
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import org.apache.logging.log4j.util.StackLocatorUtil;
+
+/**
+ * Extension of {@link java.util.logging.LogRecord} with lazy get source related methods based on Log4j's {@link StackLocatorUtil#calcLocation(String)}.
+ *
+ * @author <a href="http://www.vorburger.ch">Michael Vorburger.ch</a> for Google
+ */
+/* package-local */ final class LazyLog4jLogRecord extends LogRecord {
Review comment:
This comment is not needed IMO.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak commented on a change in pull request #698: Support source class & method in to-jul (LOG4J2-3282)
Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #698:
URL: https://github.com/apache/logging-log4j2/pull/698#discussion_r785083277
##########
File path: log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/LazyLog4jLogRecord.java
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.logging.log4j.tojul;
+
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import org.apache.logging.log4j.util.StackLocatorUtil;
+
+/**
+ * Extension of {@link java.util.logging.LogRecord} with lazy get source related methods based on Log4j's {@link StackLocatorUtil#calcLocation(String)}.
+ */
+final class LazyLog4jLogRecord extends LogRecord {
+
+ private static final long serialVersionUID = 6798134264543826471L;
+
+ // parent class LogRecord already has a needToInferCaller but it's private
+ private transient boolean inferCaller = true;
+
+ private final String fqcn;
+
+ LazyLog4jLogRecord(String fqcn, Level level, String msg) {
+ super(level, msg);
+ this.fqcn = fqcn;
+ }
+
+ @Override
+ public String getSourceClassName() {
+ if (inferCaller) {
+ inferCaller();
+ }
+ return super.getSourceClassName();
+ }
+
+ @Override
+ public String getSourceMethodName() {
+ if (inferCaller) {
+ inferCaller();
+ }
+ return super.getSourceMethodName();
+ }
+
+ private synchronized void inferCaller() {
+ StackTraceElement location = StackLocatorUtil.calcLocation(fqcn);
+ setSourceClassName(location.getClassName());
+ setSourceMethodName(location.getMethodName());
Review comment:
:+1:
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] vorburger commented on a change in pull request #698: Support source class & method in to-jul (LOG4J2-3282)
Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #698:
URL: https://github.com/apache/logging-log4j2/pull/698#discussion_r785068359
##########
File path: log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/LazyLog4jLogRecord.java
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.logging.log4j.tojul;
+
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import org.apache.logging.log4j.util.StackLocatorUtil;
+
+/**
+ * Extension of {@link java.util.logging.LogRecord} with lazy get source related methods based on Log4j's {@link StackLocatorUtil#calcLocation(String)}.
+ *
+ * @author <a href="http://www.vorburger.ch">Michael Vorburger.ch</a> for Google
Review comment:
Sure, removed. I noticed there is already an entry for https://issues.apache.org/jira/browse/LOG4J2-3282 in `changes.xml`, so from my side we could consider this simply an improvement on that, without an additional entry. I'll raise a separate PR re. `<contributors>` in `pom.xml`.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] vorburger commented on a change in pull request #698: Support source class & method in to-jul (LOG4J2-3282)
Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #698:
URL: https://github.com/apache/logging-log4j2/pull/698#discussion_r785078943
##########
File path: log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/LazyLog4jLogRecord.java
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.logging.log4j.tojul;
+
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import org.apache.logging.log4j.util.StackLocatorUtil;
+
+/**
+ * Extension of {@link java.util.logging.LogRecord} with lazy get source related methods based on Log4j's {@link StackLocatorUtil#calcLocation(String)}.
+ */
+final class LazyLog4jLogRecord extends LogRecord {
+
+ private static final long serialVersionUID = 6798134264543826471L;
+
+ // parent class LogRecord already has a needToInferCaller but it's private
+ private transient boolean inferCaller = true;
+
+ private final String fqcn;
+
+ LazyLog4jLogRecord(String fqcn, Level level, String msg) {
+ super(level, msg);
+ this.fqcn = fqcn;
+ }
+
+ @Override
+ public String getSourceClassName() {
+ if (inferCaller) {
+ inferCaller();
+ }
+ return super.getSourceClassName();
+ }
+
+ @Override
+ public String getSourceMethodName() {
+ if (inferCaller) {
+ inferCaller();
+ }
+ return super.getSourceMethodName();
+ }
+
+ private synchronized void inferCaller() {
+ StackTraceElement location = StackLocatorUtil.calcLocation(fqcn);
+ setSourceClassName(location.getClassName());
+ setSourceMethodName(location.getMethodName());
Review comment:
Done, hope looks about right like that?
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] vorburger commented on a change in pull request #698: Support source class & method in to-jul (LOG4J2-3282)
Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #698:
URL: https://github.com/apache/logging-log4j2/pull/698#discussion_r785153033
##########
File path: log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/LazyLog4jLogRecord.java
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.logging.log4j.tojul;
+
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import org.apache.logging.log4j.util.StackLocatorUtil;
+
+/**
+ * Extension of {@link java.util.logging.LogRecord} with lazy get source related methods based on Log4j's {@link StackLocatorUtil#calcLocation(String)}.
+ */
+final class LazyLog4jLogRecord extends LogRecord {
+
+ private static final long serialVersionUID = 6798134264543826471L;
+
+ // parent class LogRecord already has a needToInferCaller but it's private
+ private transient boolean inferCaller = true;
+
+ private final String fqcn;
+
+ LazyLog4jLogRecord(String fqcn, Level level, String msg) {
+ super(level, msg);
+ this.fqcn = fqcn;
+ }
+
+ @Override
+ public String getSourceClassName() {
+ if (inferCaller) {
+ inferCaller();
+ }
+ return super.getSourceClassName();
+ }
+
+ @Override
+ public String getSourceMethodName() {
+ if (inferCaller) {
+ inferCaller();
+ }
+ return super.getSourceMethodName();
+ }
+
+ private synchronized void inferCaller() {
Review comment:
OK, `synchronized` removed again now.
> I don't think `inferCaller` is volatile, no change needed there!
Duh, on TGIF the `transient` morphed into a `volatile` in my mind 😈
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] garydgregory commented on a change in pull request #698: Support source class & method in to-jul (LOG4J2-3282)
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #698:
URL: https://github.com/apache/logging-log4j2/pull/698#discussion_r785018771
##########
File path: log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/LazyLog4jLogRecord.java
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.logging.log4j.tojul;
+
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import org.apache.logging.log4j.util.StackLocatorUtil;
+
+/**
+ * Extension of {@link java.util.logging.LogRecord} with lazy get source related methods based on Log4j's {@link StackLocatorUtil#calcLocation(String)}.
+ *
+ * @author <a href="http://www.vorburger.ch">Michael Vorburger.ch</a> for Google
Review comment:
Please remove author tag. You can be credit in changes.xml and/or pom.xml.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak merged pull request #698: Support source class & method in to-jul (LOG4J2-3282)
Posted by GitBox <gi...@apache.org>.
carterkozak merged pull request #698:
URL: https://github.com/apache/logging-log4j2/pull/698
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] garydgregory commented on pull request #698: Support source class & method in to-jul (LOG4J2-3282)
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #698:
URL: https://github.com/apache/logging-log4j2/pull/698#issuecomment-1013424236
I approved the PR but as my comments are resolved. I'll let @carterkozak give the final nod.😉
TY @vorburger
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] vorburger commented on a change in pull request #698: Support source class & method in to-jul (LOG4J2-3282)
Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #698:
URL: https://github.com/apache/logging-log4j2/pull/698#discussion_r785067137
##########
File path: log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/LazyLog4jLogRecord.java
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.logging.log4j.tojul;
+
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import org.apache.logging.log4j.util.StackLocatorUtil;
+
+/**
+ * Extension of {@link java.util.logging.LogRecord} with lazy get source related methods based on Log4j's {@link StackLocatorUtil#calcLocation(String)}.
+ *
+ * @author <a href="http://www.vorburger.ch">Michael Vorburger.ch</a> for Google
+ */
+/* package-local */ final class LazyLog4jLogRecord extends LogRecord {
+
+ private static final long serialVersionUID = 6798134264543826471L;
+
+ // parent class LogRecord already has a needToInferCaller but it's private
+ private transient boolean stillNeedToInferCaller = true;
Review comment:
Parent class `LogRecord` in the JDK already has a field `needToInferCaller` (it's private) so this seemed consisted, to me. But I'm happy to rename it to `inferCaller` - done. I do not want to _flip the boolean value and call it callerInitialized_ because that would make it inconsistent with the parent class.
##########
File path: log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/LazyLog4jLogRecord.java
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.logging.log4j.tojul;
+
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import org.apache.logging.log4j.util.StackLocatorUtil;
+
+/**
+ * Extension of {@link java.util.logging.LogRecord} with lazy get source related methods based on Log4j's {@link StackLocatorUtil#calcLocation(String)}.
+ *
+ * @author <a href="http://www.vorburger.ch">Michael Vorburger.ch</a> for Google
+ */
+/* package-local */ final class LazyLog4jLogRecord extends LogRecord {
+
+ private static final long serialVersionUID = 6798134264543826471L;
+
+ // parent class LogRecord already has a needToInferCaller but it's private
+ private transient boolean stillNeedToInferCaller = true;
+
+ private final String fqcn;
+
+ LazyLog4jLogRecord(String fqcn, Level level, String msg) {
+ super(level, msg);
+ this.fqcn = fqcn;
+ }
+
+ @Override
+ public String getSourceClassName() {
+ if (stillNeedToInferCaller) {
+ inferCaller();
+ }
+ return super.getSourceClassName();
+ }
+
+ @Override
+ public String getSourceMethodName() {
+ if (stillNeedToInferCaller) {
+ inferCaller();
+ }
+ return super.getSourceMethodName();
+ }
+
+ private void inferCaller() {
Review comment:
Sure, done.
##########
File path: log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/LazyLog4jLogRecord.java
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.logging.log4j.tojul;
+
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import org.apache.logging.log4j.util.StackLocatorUtil;
+
+/**
+ * Extension of {@link java.util.logging.LogRecord} with lazy get source related methods based on Log4j's {@link StackLocatorUtil#calcLocation(String)}.
+ *
+ * @author <a href="http://www.vorburger.ch">Michael Vorburger.ch</a> for Google
+ */
+/* package-local */ final class LazyLog4jLogRecord extends LogRecord {
Review comment:
Sure, 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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak commented on a change in pull request #698: Support source class & method in to-jul (LOG4J2-3282)
Posted by GitBox <gi...@apache.org>.
carterkozak commented on a change in pull request #698:
URL: https://github.com/apache/logging-log4j2/pull/698#discussion_r785072734
##########
File path: log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/LazyLog4jLogRecord.java
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.logging.log4j.tojul;
+
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import org.apache.logging.log4j.util.StackLocatorUtil;
+
+/**
+ * Extension of {@link java.util.logging.LogRecord} with lazy get source related methods based on Log4j's {@link StackLocatorUtil#calcLocation(String)}.
+ */
+final class LazyLog4jLogRecord extends LogRecord {
+
+ private static final long serialVersionUID = 6798134264543826471L;
+
+ // parent class LogRecord already has a needToInferCaller but it's private
+ private transient boolean inferCaller = true;
+
+ private final String fqcn;
+
+ LazyLog4jLogRecord(String fqcn, Level level, String msg) {
+ super(level, msg);
+ this.fqcn = fqcn;
+ }
+
+ @Override
+ public String getSourceClassName() {
+ if (inferCaller) {
+ inferCaller();
+ }
+ return super.getSourceClassName();
+ }
+
+ @Override
+ public String getSourceMethodName() {
+ if (inferCaller) {
+ inferCaller();
+ }
+ return super.getSourceMethodName();
+ }
+
+ private synchronized void inferCaller() {
Review comment:
I don't think synchronized is necessary based on how this class is used and the thread-safety of the parent class. Synchronization here doesn't buy us anything because it only guards the method itself, not the `inferCaller` check elsewhere (which isn't volatile, nor do I think it should be) so it would only prevent additional concurrent calls from running in parallel.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] vorburger commented on pull request #698: Support source class & method in to-jul (LOG4J2-3282)
Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #698:
URL: https://github.com/apache/logging-log4j2/pull/698#issuecomment-1013361743
> Please see my comments/questions. TY!
Done!
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] carterkozak commented on pull request #698: Support source class & method in to-jul (LOG4J2-3282)
Posted by GitBox <gi...@apache.org>.
carterkozak commented on pull request #698:
URL: https://github.com/apache/logging-log4j2/pull/698#issuecomment-1013278874
> @carterkozak is this what you meant in our conversation here?
Yessir! Looks great :-) I probably could have described it more clearly.
Just needs a license header to appease rat.
We might consider naming the record type something more descriptive like `Log4jLogRecord`, but I defer to your judgement.
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] vorburger commented on a change in pull request #698: Support source class & method in to-jul (LOG4J2-3282)
Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #698:
URL: https://github.com/apache/logging-log4j2/pull/698#discussion_r785079189
##########
File path: log4j-to-jul/src/main/java/org/apache/logging/log4j/tojul/LazyLog4jLogRecord.java
##########
@@ -0,0 +1,62 @@
+/*
+ * 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.logging.log4j.tojul;
+
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
+import org.apache.logging.log4j.util.StackLocatorUtil;
+
+/**
+ * Extension of {@link java.util.logging.LogRecord} with lazy get source related methods based on Log4j's {@link StackLocatorUtil#calcLocation(String)}.
+ */
+final class LazyLog4jLogRecord extends LogRecord {
+
+ private static final long serialVersionUID = 6798134264543826471L;
+
+ // parent class LogRecord already has a needToInferCaller but it's private
+ private transient boolean inferCaller = true;
+
+ private final String fqcn;
+
+ LazyLog4jLogRecord(String fqcn, Level level, String msg) {
+ super(level, msg);
+ this.fqcn = fqcn;
+ }
+
+ @Override
+ public String getSourceClassName() {
+ if (inferCaller) {
+ inferCaller();
+ }
+ return super.getSourceClassName();
+ }
+
+ @Override
+ public String getSourceMethodName() {
+ if (inferCaller) {
+ inferCaller();
+ }
+ return super.getSourceMethodName();
+ }
+
+ private synchronized void inferCaller() {
Review comment:
s/ourselves/yourselves/
--
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: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org