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 2021/08/21 19:20:57 UTC
[GitHub] [logging-log4j2] redapel opened a new pull request #564: LOG4J2-3122 fix Logging to Log4j2 MongoDb4 Appender get ERROR on trace, debug and info level.
redapel opened a new pull request #564:
URL: https://github.com/apache/logging-log4j2/pull/564
After do some debugging, i found out that there are some part of the code that is missing in MongoDb4Provider class when creating log4j-mongodb4 module, knowing that this part of the code is exists in log4j-mongodb3 module. The code that check if the database actually requires authentication. I don't know if that is intentional or not but somehow this part of the code is what making the old log4j-mongodb3 module running without error. So i just copy the old code to the new log4j-mongodb4 module, and now it is running without error to. What do you think about this log4j2 team?
Please respond.
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] redapel commented on a change in pull request #564: LOG4J2-3122 fix Logging to Log4j2 MongoDb4 Appender get ERROR on trace, debug and info level.
Posted by GitBox <gi...@apache.org>.
redapel commented on a change in pull request #564:
URL: https://github.com/apache/logging-log4j2/pull/564#discussion_r697823808
##########
File path: log4j-mongodb4/pom.xml
##########
@@ -88,6 +88,17 @@
<artifactId>de.flapdoodle.embed.mongo</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.logging.log4j</groupId>
+ <artifactId>log4j-slf4j-impl</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>com.github.stefanbirkner</groupId>
+ <artifactId>system-rules</artifactId>
Review comment:
I already remove the use of system-rules library
--
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 #564: LOG4J2-3122 fix Logging to Log4j2 MongoDb4 Appender get ERROR on trace, debug and info level.
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #564:
URL: https://github.com/apache/logging-log4j2/pull/564#issuecomment-903173794
> After do some debugging, i found out that there are some part of the code that is missing in MongoDb4Provider class when creating log4j-mongodb4 module, knowing that this part of the code is exists in log4j-mongodb3 module. The code that check if the database actually requires authentication. I don't know if that is intentional or not but somehow this part of the code is what making the old log4j-mongodb3 module running without error. So i just copy the old code to the new log4j-mongodb4 module, and now it is running without error to. What do you think about this log4j2 team?
> Please respond.
> Thanks.
Hi @redapel
Yes, it was intentional, with the version 4 driver, anything should be configurable from the connection string, unlike with the version 3 driver IIRC.
In general, you'll want to provide tests for new code. If you'd included a test with a configuration file, we could have determined if that was the code... so it still might be the case, but, no test...
--
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] redapel commented on a change in pull request #564: LOG4J2-3122 fix Logging to Log4j2 MongoDb4 Appender get ERROR on trace, debug and info level.
Posted by GitBox <gi...@apache.org>.
redapel commented on a change in pull request #564:
URL: https://github.com/apache/logging-log4j2/pull/564#discussion_r697825524
##########
File path: log4j-mongodb4/pom.xml
##########
@@ -88,6 +88,17 @@
<artifactId>de.flapdoodle.embed.mongo</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.logging.log4j</groupId>
+ <artifactId>log4j-slf4j-impl</artifactId>
Review comment:
The test is about using slf4j on top of log4j2 with mongodb4 appender. By using slf4j without adding that dependency, the log message (Hello log) will not get written to the mongodb database so the test would fail. Also there are warning message being shown :
`SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".`
`SLF4J: Defaulting to no-operation (NOP) logger implementation`
`SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.`
Do i need to remove that added dependency or something?
--
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] redapel commented on pull request #564: LOG4J2-3122 fix Logging to Log4j2 MongoDb4 Appender get ERROR on trace, debug and info level.
Posted by GitBox <gi...@apache.org>.
redapel commented on pull request #564:
URL: https://github.com/apache/logging-log4j2/pull/564#issuecomment-906654798
Hi team,
Please have take a look to the new test case that i submitted. In that test, i choose to use real MongoDB server instead of the Flapdoodle's embedded MongoDB as the later can't be considered as "full integration testing", there are possibilities that both of them behaves differently, so it is better to use solution like docker container. By the way, i run the test using a replica set cluster with only single server act as a PRIMARY. I suggest you to run the test more than one (for example 10 times), Since there are at least 2 different error cases produced when i try to test it. Both of them i already made an assertion to fail the test. Let me know about the progress.
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] redapel commented on pull request #564: LOG4J2-3122 fix Logging to Log4j2 MongoDb4 Appender get ERROR on trace, debug and info level.
Posted by GitBox <gi...@apache.org>.
redapel commented on pull request #564:
URL: https://github.com/apache/logging-log4j2/pull/564#issuecomment-907585610
> > Hi team,
> > Please have take a look to the new test case that i submitted. In that test, i choose to use real MongoDB server instead of the Flapdoodle's embedded MongoDB as the later can't be considered as "full integration testing", there are possibilities that both of them behaves differently, so it is better to use solution like docker container. By the way, i run the test using a replica set cluster with only single server act as a PRIMARY. I suggest you to run the test more than one (for example 10 times), Since there are at least 2 different error cases produced when i try to test it. Both of them i already made an assertion to fail the test. Let me know about the progress.
> > Thanks.
>
> You'll want to fit in our existing test framework.
I already change to Flapdoodle's embedded MongoDB
--
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 #564: LOG4J2-3122 fix Logging to Log4j2 MongoDb4 Appender get ERROR on trace, debug and info level.
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #564:
URL: https://github.com/apache/logging-log4j2/pull/564#discussion_r697036474
##########
File path: log4j-mongodb4/pom.xml
##########
@@ -88,6 +88,17 @@
<artifactId>de.flapdoodle.embed.mongo</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.logging.log4j</groupId>
+ <artifactId>log4j-slf4j-impl</artifactId>
Review comment:
Why is slf4j involved?
--
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 #564: LOG4J2-3122 fix Logging to Log4j2 MongoDb4 Appender get ERROR on trace, debug and info level.
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #564:
URL: https://github.com/apache/logging-log4j2/pull/564#issuecomment-906799734
> Hi team,
> Please have take a look to the new test case that i submitted. In that test, i choose to use real MongoDB server instead of the Flapdoodle's embedded MongoDB as the later can't be considered as "full integration testing", there are possibilities that both of them behaves differently, so it is better to use solution like docker container. By the way, i run the test using a replica set cluster with only single server act as a PRIMARY. I suggest you to run the test more than one (for example 10 times), Since there are at least 2 different error cases produced when i try to test it. Both of them i already made an assertion to fail the test. Let me know about the progress.
> Thanks.
You'll want to fit in our existing test framework.
--
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 #564: LOG4J2-3122 fix Logging to Log4j2 MongoDb4 Appender get ERROR on trace, debug and info level.
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #564:
URL: https://github.com/apache/logging-log4j2/pull/564#discussion_r697032754
##########
File path: log4j-mongodb4/pom.xml
##########
@@ -88,6 +88,17 @@
<artifactId>de.flapdoodle.embed.mongo</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.logging.log4j</groupId>
+ <artifactId>log4j-slf4j-impl</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>com.github.stefanbirkner</groupId>
+ <artifactId>system-rules</artifactId>
Review comment:
Is this licensed under a compatible license with Apache?
--
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 #564: LOG4J2-3122 fix Logging to Log4j2 MongoDb4 Appender get ERROR on trace, debug and info level.
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #564:
URL: https://github.com/apache/logging-log4j2/pull/564#discussion_r697855364
##########
File path: log4j-mongodb4/pom.xml
##########
@@ -88,6 +88,17 @@
<artifactId>de.flapdoodle.embed.mongo</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.logging.log4j</groupId>
+ <artifactId>log4j-slf4j-impl</artifactId>
Review comment:
Can you reproduce the issue by using the log4j API? IOW, without going through the slf4j layer? If this is an issue in log4j, adding slf4j should not be needed to show the issue.
--
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