Re: [I] Creating a static NDC causes a segfault on exit in 1.3.0 [logging-log4cxx]
wrohv commented on issue #425: URL: https://github.com/apache/logging-log4cxx/issues/425#issuecomment-2460981177 I tried the linked pull request branch and it appears to solve the problem for me. Thanks! -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[I] Confusing example in Thread Context documentation (logging-log4j2)
MaxAller opened a new issue, #3177:
URL: https://github.com/apache/logging-log4j2/issues/3177
## Description
On [this
page](https://logging.apache.org/log4j/2.x/manual/thread-context.html#init)
([in
source](https://github.com/apache/logging-log4j2/blob/2.x/src/site/antora/modules/ROOT/pages/manual/thread-context.adoc#initializing-thread-context)),
this example is given:
```
LOGGER.debug("Starting background thread for user");
Map mdc = ThreadContext.getImmutableContext(); // (1)
List ndc = ThreadContext.getImmutableStack().asList(); // (1)
executor.submit(() -> {
try (CloseableThreadContext.Instance ignored = CloseableThreadContext
.putAll(values) // (2)
.pushAll(messages)) { // (2)
LOGGER.debug("Processing for user started");
// ...
}
});
```
However...there's no connection between the (1) and (2) items. Is `values`
supposed to be `mdc`, and `messages` should be `ndc`?
--
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-unsubscr...@logging.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [I] Replace field injection with setter injection in `3.x` (logging-log4j2)
jaykataria commented on issue #2769:
URL:
https://github.com/apache/logging-log4j2/issues/2769#issuecomment-2458996673
Hi @ppkarwasz
Made some good progress but needed your input on something:
I am not sure if we should add type checking here:
```
if (methodName.startsWith("set")
&& methodElement.getParameters().size() == 1) {
// Check if this is the right setter and if it matches
return
}
```
I added this code:
```
if ((methodName.toLowerCase(Locale.ROOT).startsWith("set") // Typical
pattern for setters
||
methodName.toLowerCase(Locale.ROOT).startsWith("with")) // Typical pattern for
setters
&& methodElement.getParameters().size() == 1 //
It is a weird pattern to not have public setter
) {
// Check if the method parameter matches the type.
boolean methodParamTypeMatchesBoxedFieldType = false;
TypeMirror fieldType = element.asType();
TypeMirror methodParamType =
methodElement.getParameters().get(0).asType();
Types typeUtils = processingEnv.getTypeUtils();
if (fieldType.getKind().isPrimitive() &&
!methodParamType.getKind().isPrimitive()) {
methodParamTypeMatchesBoxedFieldType =
typeUtils.isSameType(methodParamType, typeUtils.boxedClass((PrimitiveType)
fieldType).asType());
} else if (methodParamType.getKind().isPrimitive()
&& !fieldType.getKind().isPrimitive()) {
methodParamTypeMatchesBoxedFieldType =
typeUtils.isSameType(fieldType, typeUtils.boxedClass((PrimitiveType)
methodParamType).asType());
}
boolean methodParamTypeMatchesFieldType =
typeUtils.isSameType(methodParamType, fieldType) ||
methodParamTypeMatchesBoxedFieldType;
// Check if method is public
boolean isPublicMethod =
methodElement.getModifiers().contains(Modifier.PUBLIC);
boolean foundPublicSetter =
methodParamTypeMatchesFieldType && isPublicMethod;
if (foundPublicSetter) {
// Hurray we found a public setter for the field!
return;
}
}
```
But the issue that I face is with such kind of setters, we are throwing an
error in such cases due to type checking :
```
public static final class Builder implements
org.apache.logging.log4j.core.util.Builder {
@PluginBuilderAttribute
@Required(message = "No age provided for IfLastModified")
private
org.apache.logging.log4j.core.appender.rolling.action.Duration age;
public Builder setAge(final Duration age) {
this.age =
org.apache.logging.log4j.core.appender.rolling.action.Duration.ofMillis(age.toMillis());
return this;
}
```
As you can see the setAge method has a different parameter type than the
fields type.
if we do have a hard check on the naming convention "setFieldName" (This
becomes more restrictive, do not recommend, I have seen people get really
creative for no reason) that has a number of problems as well.
I do believe we should do some sort of checks, just checking for
`methodName.startsWith("set") && methodElement.getParameters().size() == 1` to
identify
Hence I have come up with the following plan:
1. method name starts with "set" or "with" and method has 1 param
2. method param type is the same as the field type (includes boxed
comparisons)
2.a if the above is not being followed then as a fall back we can check if
there is a method that follows the naming convention like in the above case
after that I believe we can give up, because then people are doing something
really creative with their code.
Hence building on the above statement, I have the following logic, which
seems to work :) :
```
if ((methodName.toLowerCase(Locale.ROOT).startsWith("set") // Typical
pattern for setters
||
methodName.toLowerCase(Locale.ROOT).startsWith("with")) // Typical pattern for
setters
&& methodElement.getParameters().size() == 1 //
It is a weird pattern to not have public setter
) {
// Check if the method parameter matches the type.
boolean methodParamTypeMatchesBoxedFieldType = false;
TypeMirror fieldType = element.asType();
TypeMirror methodParamType =
Re: [I] Replace field injection with setter injection in `3.x` (logging-log4j2)
ppkarwasz commented on issue #2769: URL: https://github.com/apache/logging-log4j2/issues/2769#issuecomment-2459096985 **Note**: I noticed that some of our builders have complex conventions for the name of the setter: the setter for an `isUnicode` field should be called `setUnicode` and **not** `setIsUnicode`. -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [2.17.1] AsyncLogger thread is stuck on 100% CPU on lmax-34. queue polling (logging-log4j2)
ppkarwasz commented on issue #2169: URL: https://github.com/apache/logging-log4j2/issues/2169#issuecomment-2459451988 @Baixiu-code, - Are you using Log4j Core `2.24.1`? - Did you try using `com.lmax:disruptor` version `4.0.0`? - What is the status of the **polling** thread? The polling thread's name starts with `Log4j2-TF--AsyncLogger` as showed in the [initial comment for this issue](#issue-2069159269). Your screenshot only covers the logging threads, which are blocked, because the queue is full. Can you provide the stack trace of the polling thread as **text** instead of an image? -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] Add SLF4J to Log4j API bridge [logging-slf4j]
ppkarwasz opened a new pull request, #1: URL: https://github.com/apache/logging-slf4j/pull/1 This adds a new `slf4j-to-log4j-api` bridge based on `log4j-slf4j2-impl` from the `2.x` branch of Log4j. -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] Add JPL to Log4j API bridge [logging-jdk]
ppkarwasz opened a new pull request, #1: URL: https://github.com/apache/logging-jdk/pull/1 This add a new `jpl-to-log4j-api` bridge based on `log4j-jpl` from the `2.x` branch of Log4j. -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] Add Log4j API to SLF4J bridge [logging-slf4j]
ppkarwasz opened a new pull request, #2: URL: https://github.com/apache/logging-slf4j/pull/2 This adds a new `log4j-api-to-slf4j` bridge based on `log4j-to-slf4j` from the `2.x` branch of Log4j. -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] Add Log4j API to JUL bridge [logging-jdk]
ppkarwasz opened a new pull request, #3: URL: https://github.com/apache/logging-jdk/pull/3 This add a new `log4j-api-to-jul` bridge based on `log4j-to-jul` from the `2.x` branch of Log4j. -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add JPL to Log4j API bridge [logging-jdk]
github-advanced-security[bot] commented on PR #1: URL: https://github.com/apache/logging-jdk/pull/1#issuecomment-2459612607 This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on [this overview](/apache/logging-jdk/security/code-scanning?query=pr%3A1+is%3Aopen). Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out [the documentation](https://docs.github.com/code-security/code-scanning/introduction-to-code-scanning/about-code-scanning). -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] Add JUL to Log4j API bridge [logging-jdk]
ppkarwasz opened a new pull request, #2: URL: https://github.com/apache/logging-jdk/pull/2 This add a new `jul-to-log4j-api` bridge based on `jul-to-log4j` from the `main` branch of Log4j. -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] Replace field injection with setter injection in `3.x` (logging-log4j2)
ppkarwasz commented on issue #2769:
URL:
https://github.com/apache/logging-log4j2/issues/2769#issuecomment-2459091744
@jaykataria,
> But the issue that I face is with such kind of setters, we are throwing an
error in such cases due to type checking:
> ```java
> private org.apache.logging.log4j.core.appender.rolling.action.Duration age;
> public Builder setAge(final Duration age) { ... }
> ```
Checking the type of the setter parameter is **not** necessary. All the
configuration attributes in Log4j are provided as `String`s anyway (e.g. in an
XML configuration file). As long as there **is** a setter, the `PluginBuilder`
can handle the conversion. Besides, the main utility of having setters is to be
able to change the type of the field, without breaking backward compatibility.
:wink:
Instead of checking the type of the parameter, it would be more useful to
check the **return type** of the setter: the return types of our setters are
**not** always **equal** to the type of the builder (e.g. they can be type
variables for abstract builders), but the return type should be **assignable**
to the builder type. I believe that
[`Types.isAssignable()`](https://docs.oracle.com/javase/8/docs/api/javax/lang/model/util/Types.html#isAssignable-javax.lang.model.type.TypeMirror-javax.lang.model.type.TypeMirror-)
should handle that, although I am not sure how well it manages parameterized
types.
--
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-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Add SLF4J to Log4j API bridge [logging-slf4j]
github-advanced-security[bot] commented on PR #1: URL: https://github.com/apache/logging-slf4j/pull/1#issuecomment-2459861207 This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on [this overview](/apache/logging-slf4j/security/code-scanning?query=pr%3A1+is%3Aopen). Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out [the documentation](https://docs.github.com/code-security/code-scanning/introduction-to-code-scanning/about-code-scanning). -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] [2.17.1] AsyncLogger thread is stuck on 100% CPU on lmax-34. queue polling (logging-log4j2)
Baixiu-code commented on issue #2169: URL: https://github.com/apache/logging-log4j2/issues/2169#issuecomment-2459214534 same issues. and cpu 100%. and t version is 3.4.2 .  -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] Replace field injection with setter injection in `3.x` (logging-log4j2)
jaykataria commented on issue #2769:
URL:
https://github.com/apache/logging-log4j2/issues/2769#issuecomment-2461336252
> Note: I noticed that some of our builders have complex conventions for the
name of the setter: the setter for an isUnicode field should be called
setUnicode and not setIsUnicode.
+1. I removed the type check and kept the naming check and fix the
compilation issues for the purpose. I had to change the name of some fields due
to this. It should be a backwards compatible change.
>Instead of checking the type of the parameter, it would be more useful to
check the return type of the setter: the return types of our setters are not
always equal to the type of the builder (e.g. they can be type variables for
abstract builders), but the return type should be assignable to the builder
type. I believe that
[Types.isAssignable()](https://docs.oracle.com/javase/8/docs/api/javax/lang/model/util/Types.html#isAssignable-javax.lang.model.type.TypeMirror-javax.lang.model.type.TypeMirror-)
should handle that, although I am not sure how well it manages parameterized
types
I did incorporate that in code:
Some of the methods had a void return type I fixed that for example changed
[SocketPerformancePreferences](https://github.com/apache/logging-log4j2/blob/d229eda077c313a8a2060caa42531f6b539b1a09/log4j-core/src/main/java/org/apache/logging/log4j/core/net/SocketPerformancePreferences.java#L78-L88)
```
public SocketPerformancePreferences setBandwidth(final int bandwidth) {
this.bandwidth = bandwidth;
return this;
}
public SocketPerformancePreferences setConnectionTime(final int
connectionTime) {
this.connectionTime = connectionTime;
return this;
}
public SocketPerformancePreferences setLatency(final int latency) {
this.latency = latency;
return this;
}
```
The issue is that, the check-api-comp build task fails for the purpose when
I do the code change, should we ignore it? All of my changes so far are
backwards compatible, but the tool thinks that these are "MAJOR" changes.
I have created a dummy PR in my own repo: [Code
Changes](https://github.com/jaykataria/logging-log4j2/pull/1/commits/44695395e86b1439e15fbe778e2d58a0dc5037c6)
to show you the code changes and how they should be backward compatible,
please let me know if that works, also I am new to github, so learning its
quirks, My company uses its own system, so workflows code reviewing are a bit
different. So please bear with me if it is not a fancy PR and just my repo, as
long as you can see the commit and the diff that is all that matters to me :)
>Besides, the main utility of having setters is to be able to change the
type of the field, without breaking backward compatibility.
😃 , yeah it is interesting to see, this sort of usage of setters, I must say
I wasn't acquainted it with it, which just gives me more breadth of knowledge.
--
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-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [I] Replace field injection with setter injection in `3.x` (logging-log4j2)
ppkarwasz commented on issue #2769: URL: https://github.com/apache/logging-log4j2/issues/2769#issuecomment-2461500874 @jaykataria, Great job! :100: > > Note: I noticed that some of our builders have complex conventions for the name of the setter: the setter for an isUnicode field should be called setUnicode and not setIsUnicode. > > +1. I removed the type check and kept the naming check and fix the compilation issues for the purpose. I had to change the name of some fields due to this. It should be a backwards compatible change. Unfortunately, changing the name of `@PluginBuilderAttribute`-annotated fields **is** a breaking change, because those names are used in the configuration file. What you should do in this case is **deprecate** the old setter and create a new one. In PR #3174 I fixed the setter problems I found. > Some of the methods had a void return type I fixed that for example changed [SocketPerformancePreferences](https://github.com/apache/logging-log4j2/blob/d229eda077c313a8a2060caa42531f6b539b1a09/log4j-core/src/main/java/org/apache/logging/log4j/core/net/SocketPerformancePreferences.java#L78-L88) > > The issue is that, the check-api-comp build task fails for the purpose when I do the code change, should we ignore it? All of my changes so far are backwards compatible, but the tool thinks that these are "MAJOR" changes. Nice catch! Unfortunately changing any public signature **is** technically a breaking change. Of course the ultimate classification is up to the developers. We have two choices here: 1. Add a configuration option to the processor to change the behavior for mismatched return types from `ERROR` to `WARNING`. 2. Tell BND Baseline to ignore those changes. I am quite strict on binary compatibility, even if it concerns code that probably nobody uses. @vy, @JWT007, what do you think? Is fixing those broken setters in `SocketPerformancePreferences` worth the breaking change? -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] Creating a static NDC causes a segfault on exit in 1.3.0 [logging-log4cxx]
swebb2066 closed issue #425: Creating a static NDC causes a segfault on exit in 1.3.0 URL: https://github.com/apache/logging-log4cxx/issues/425 -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Prevent abnormal termination on exit when using a static NDC/MDC variable [logging-log4cxx]
swebb2066 merged PR #426: URL: https://github.com/apache/logging-log4cxx/pull/426 -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [I] Confusing example in Thread Context documentation (logging-log4j2)
ppkarwasz closed issue #3177: Confusing example in Thread Context documentation URL: https://github.com/apache/logging-log4j2/issues/3177 -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Remove `log4j-core` dependency from SLF4J bridges (logging-log4j2)
ppkarwasz closed pull request #3038: Remove `log4j-core` dependency from SLF4J bridges URL: https://github.com/apache/logging-log4j2/pull/3038 -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Remove `log4j-core` dependency from SLF4J bridges (logging-log4j2)
ppkarwasz commented on PR #3038: URL: https://github.com/apache/logging-log4j2/pull/3038#issuecomment-2460862741 Closing this, since we discussed an [alternative solution on `dev@logging`](https://lists.apache.org/thread/tk8k8bnmy11b08xftr4p37yzgghpf61j). -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Removes bridges from Log4j 3 (logging-log4j2)
ppkarwasz commented on PR #2924: URL: https://github.com/apache/logging-log4j2/pull/2924#issuecomment-2460865426 As [discussed on `dev@logging`](https://lists.apache.org/thread/tk8k8bnmy11b08xftr4p37yzgghpf61j) all the bridges will be removed from Log4j Core 3.x, since they don't belong here. They will be replaced with unambiguously named artifacts. While we discuss where to put those artifacts, a draft version is available in these PRs: - [ ] apache/logging-jdk#1 - [ ] apache/logging-jdk#2 - [ ] apache/logging-jdk#3 - [ ] apache/logging-slf4j#1 - [ ] apache/logging-slf4j#2 **Note**: the "old" 2.x artifacts are referenced from `log4j-bom` in the branch 3.x and can be used without problems with Log4j Core 3.x or any other Log4j API implementation. -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Removes bridges from Log4j 3 (logging-log4j2)
ppkarwasz merged PR #2924: URL: https://github.com/apache/logging-log4j2/pull/2924 -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
