You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "tonycody (via GitHub)" <gi...@apache.org> on 2024/03/19 08:15:58 UTC

[I] NoSqlDatabaseManager createObject Extended Support (logging-log4j2)

tonycody opened a new issue, #2389:
URL: https://github.com/apache/logging-log4j2/issues/2389

   org.apache.logging.log4j.core.appender.nosql.NoSqlConnection#createObject
   
   This interface method, currently does not pass down the LogEvent from NoSqlDatabaseManager, and in practice, certain scenarios require the original LogEvent object. Instead of passing down org.apache.logging.log4j.core.appender.nosql.NoSqlDatabaseManager#setFields(org.apache.logging.log4j.core.LogEvent, org.apache.logging.log4j.core. .logging.log4j.core.appender.nosql.NoSqlObject<W>) method converted.
   
   Therefore, it is proposed to extend org.apache.logging.log4j.core.appender.nosql.NoSqlConnection#createObject, which also supports user-defined org.apache.logging.log4j.core.appender. .nosql.NoSqlConnection#createObject interface to the original LogEvent object.


-- 
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.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [I] Pass `LogEvent` to `NoSqlConnection#insertObject` (logging-log4j2)

Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on issue #2389:
URL: https://github.com/apache/logging-log4j2/issues/2389#issuecomment-2009704798

   @tonycody, the request sounds legit to me. Would you mind providing a bit more context, please?
   1. Which Log4j version are you using?
   2. Which appender are you using? (If it is a custom one, what is the sink it is writing to?)
   3. What is the use case necessitating the `LogEvent` in `insertObject()`?


-- 
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


Re: [I] Pass `LogEvent` to `NoSqlConnection#insertObject` (logging-log4j2)

Posted by "tonycody (via GitHub)" <gi...@apache.org>.
tonycody commented on issue #2389:
URL: https://github.com/apache/logging-log4j2/issues/2389#issuecomment-2011722823

   > > My scenario is to write the stack information resolved by the method java.lang.Throwable#lockedPrintStackTrace to the NoSql data source. Therefore, I need to get the Throwable object when implementing the log4j NoSql Provider.
   > 
   > If I understand you correctly, you don't like the way this code serializes the `Throwable`:
   > 
   > https://github.com/apache/logging-log4j2/blob/67479ee93c94038f43b10081f3d86b742b4ef0f5/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/nosql/NoSqlDatabaseManager.java#L217-L237
   > 
   > and you want to do it in a different way?
   > 
   > There is **already** a hook for that: a `NoSqlAppender` can also have a [`Layout`](https://logging.apache.org/log4j/2.x/javadoc/log4j-core/org/apache/logging/log4j/core/Layout.html) component. All you need is a layout that transforms `LogEvent` into whatever you want:
   > 
   > ```java
   > @Plugin(name = "MyNoSqlLayout", category = Node.CATEGORY)
   > public class MyNoSqlLayout extends AbstractLayout<MapMessage<?, ?>> {
   > 
   >     private Map<String, ?> toMap(LogEvent event) {
   >         /*
   >          * Put your logic here.
   >          */
   >     }
   > 
   >     public MyNoSqlLayout(Configuration configuration) {
   >         super(configuration, null, null);
   >     }
   > 
   >     @Override
   >     public byte[] toByteArray(LogEvent event) {
   >         return toSerializable(event).getFormattedMessage().getBytes();
   >     }
   > 
   >     @Override
   >     public MapMessage<?, ?> toSerializable(LogEvent event) {
   >         return new MapMessage<>(toMap(event));
   >     }
   > 
   >     @Override
   >     public String getContentType() {
   >         return "text/plain";
   >     }
   > }
   > ```
   > 
   > and add it to the `NoSql` appender:
   > 
   > ```
   > <NoSql name="NoSql">
   >     ...
   >     <MyNoSqlLayout/>
   > </NoSql>
   > ```
   
   We have considered this scenario, but this scenario means that the user must use a custom Layout! Essentially, adding LogEvent to an interface or NoSqlObject to be passed through is not a big deal, from a global point of view.LogEvent should be supported outside of the NoSqlManager extension, since Appender itself has it, so why would NoSqlManger extend Appender after is not supported after NoSqlManger extends Appender?


-- 
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


Re: [I] Pass `LogEvent` to `NoSqlConnection#insertObject` (logging-log4j2)

Posted by "tonycody (via GitHub)" <gi...@apache.org>.
tonycody commented on issue #2389:
URL: https://github.com/apache/logging-log4j2/issues/2389#issuecomment-2011325316

   @vy I looked at the log4j redis Appender mentioned above, and it's actually a different scenario. The implementation mentioned above is based on Appender, which also uses LogEvent, but the Appender interface provides LogEvent.
   
   It's interesting to note that the NoSql extension is actually a sub-implementation of the Appender class, but it's categorised in the form of NoSql, but it doesn't include the LogEvent pass.
   
   This doesn't really make sense to me!
   
   It feels more logical to provide a method to pass LogEvent in org.apache.logging.log4j.core.appender.nosql.NoSqlObject.
   Instead, Appender filters out LogEvent passing at the NoSql level.


-- 
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


Re: [I] Pass `LogEvent` to `NoSqlConnection#insertObject` (logging-log4j2)

Posted by "tonycody (via GitHub)" <gi...@apache.org>.
tonycody closed issue #2389: Pass `LogEvent` to `NoSqlConnection#insertObject`
URL: https://github.com/apache/logging-log4j2/issues/2389


-- 
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


Re: [I] Pass `LogEvent` to `NoSqlConnection#insertObject` (logging-log4j2)

Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on issue #2389:
URL: https://github.com/apache/logging-log4j2/issues/2389#issuecomment-2011855640

   > We have considered this scenario, but this scenario means that the user must use a custom Layout! Essentially, adding LogEvent to an interface or NoSqlObject to be passed through is not a big deal, from a global point of view.LogEvent should be supported outside of the NoSqlManager extension, since Appender itself has it, so why would NoSqlManger extend Appender after is not supported after NoSqlManger extends Appender?
   
   [`NoSqlManager` does **not** implement the `Appender` interface, `NoSqlAppender` does]
   
   There is a separation of concerns between an `AbstractManager` and `Layout`: the layout transforms the event into a form suitable for the manager (e.g. `byte[]` for managers based on `OutputStream` or `MapMessage` for `NoSqlManager), while the manager only needs to persist the data to permanent storage.
   
   Although all appenders have a default layout, users of output stream based appenders almost always specify their own layout. I don't think that users of NoSQL-based appenders would be surprised by this fact.
   
   If you don't like the form of the default layout used by `NoSqlAppender` (which is essentially the same as those used by `XmlLayout`, `JsonLayout` and `YamlLayout`), please [open a discussion](https://github.com/apache/logging-log4j2/discussions) to allow other users to express their preferences. There are certainly more standard ways to represent a logging event than a serialization of the `LogEvent` interface.


-- 
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


Re: [I] Pass `LogEvent` to `NoSqlConnection#insertObject` (logging-log4j2)

Posted by "tonycody (via GitHub)" <gi...@apache.org>.
tonycody commented on issue #2389:
URL: https://github.com/apache/logging-log4j2/issues/2389#issuecomment-2019057713

   👌Thank you!


-- 
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


Re: [I] Pass `LogEvent` to `NoSqlConnection#insertObject` (logging-log4j2)

Posted by "tonycody (via GitHub)" <gi...@apache.org>.
tonycody commented on issue #2389:
URL: https://github.com/apache/logging-log4j2/issues/2389#issuecomment-2011629770

   > @tonycody,
   > 
   > I think we hit an [XY problem](https://en.wikipedia.org/wiki/XY_problem) in this discussion. Could you explain plainly (without technical details) what are you trying to achieve?
   > 
   > From you code example I can not guess whether:
   > 
   > * you want to modify the JSON-y object, before it is sent to the database?
   > * you want to observe the events that are logged and perform some action based on them?
   > * you are not satisfied with the way a `Throwable` is converted to JSON?
   > * you want to make your plugin usable by users of other NoSQL databases?
   > 
   > Let's focus on the origin of the problem and only then choose the best technical solution.
   > 
   > > @vy I looked at the log4j redis Appender mentioned above, and it's actually a different scenario. The implementation mentioned above is based on Appender, which also uses LogEvent, but the Appender interface provides LogEvent.
   > 
   > To send log events to a resource you need to implement `Appender`. It is also advised to extend `AbstractManager` to prevent your appender from losing events during reconfiguration. The reset is pretty much up to you.
   > 
   > > It's interesting to note that the NoSql extension is actually a sub-implementation of the Appender class, but it's categorised in the form of NoSql, but it doesn't include the LogEvent pass.
   > > This doesn't really make sense to me!
   > 
   > Yes, `NoSqlAppender` is an implementation of `Appender` with a **fixed** way to convert a log event into a JSON-y object. Similarly to `XmlLayout`, `HtmlLayout`, `JsonLayout` and `YamlLayout` (but unlike `JsonTemplateLayout`) it gives you only a limited set of configuration options regarding the formatting of the event.
   
   My scenario is to write the stack information resolved by the method java.lang.Throwable#lockedPrintStackTrace to the NoSql data source. Therefore, I need to get the Throwable object when implementing the log4j NoSql Provider.
   
   Currently, the NoSqlDatabaseManager is based on Appender implementation and then the LogEvent is lost. As a result, the class that implements NoSql can't get the LogEvent object, so it can't get the Throwable object!


-- 
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


Re: [I] Pass `LogEvent` to `NoSqlConnection#insertObject` (logging-log4j2)

Posted by "tonycody (via GitHub)" <gi...@apache.org>.
tonycody commented on issue #2389:
URL: https://github.com/apache/logging-log4j2/issues/2389#issuecomment-2009772986

   > > I'm implementing a NoSqlConnection where the received data is written to Redis. I need the original thrown object so that I can use java.lang.Throwable#printStackTrace(java.io.PrintWriter) to get the StrackTrace and convert it to String via Stream, which makes it a lot less work for me. Although StrackTrace is included in the current HashMap, I need to do some complicated parsing into String format, whereas printStackTrace is natively supported and more consistent.
   > 
   > `NoSqlDatabaseManager#writeInternal` is implemented as follows:
   > 
   > ```
   > protected void writeInternal(final LogEvent event, final Serializable serializable) {
   >     if (!this.isRunning() || this.connection == null || this.connection.isClosed()) {
   >         throw new AppenderLoggingException(
   >                 "Cannot write logging event; NoSQL manager not connected to the database.");
   >     }
   > 
   >     final NoSqlObject<W> entity = this.connection.createObject();
   >     if (serializable instanceof MapMessage) {
   >         setFields((MapMessage<?, ?>) serializable, entity);
   >     } else {
   >         setFields(event, entity);
   >     }
   >     setAdditionalFields(entity);
   >     this.connection.insertObject(entity);
   > }
   > ```
   > 
   > Why don't you simply use `setFields()` to capture the `Throwable` and use it in `insertObject()`?
   > 
   > I am also curious, why did you opt for implementing your own Redis appender instead of using [log4j2-redis-appender](/vy/log4j2-redis-appender)?
   
   The usage scenarios are different, another one I'm considering. Bringing LogEvent out doesn't affect the existing design, one more option for custom implementation classes. There are some other extensions that can be done with LogEvent.


-- 
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


Re: [I] Pass `LogEvent` to `NoSqlConnection#insertObject` (logging-log4j2)

Posted by "tonycody (via GitHub)" <gi...@apache.org>.
tonycody commented on issue #2389:
URL: https://github.com/apache/logging-log4j2/issues/2389#issuecomment-2011260228

   > I am not keen on adding new methods based on speculative functionality. Could you give a more concrete example demonstrating without a new `insertObject(LogEvent,Entity)`, it is really difficult to carry out a certain business logic, please?
   > 
   > **Background:** @tonycody, I am very thankful that you reach out to us. You did not only create a ticket, but also submitted a PR. Much appreciated. Please understand that every single line we add to the public API becomes a liability and maintenance burden we need to carry for decades. Given that the Log4j API is already pretty bloated, we need to justify every single new thing we are about to add.
   
   I understand, as I said above. Adding this feature does not affect the current framework design. But it adds a lot of possibilities for extensions.


-- 
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


Re: [I] Pass `LogEvent` to `NoSqlConnection#insertObject` (logging-log4j2)

Posted by "tonycody (via GitHub)" <gi...@apache.org>.
tonycody commented on issue #2389:
URL: https://github.com/apache/logging-log4j2/issues/2389#issuecomment-2011270792

   @vy 
   e.g.
   ```java
   @Override 
       public void insertObject(LogEvent event, NoSqlObject<Map<String, Object>> object) {
           LogMetaData  logMeta= LogMapper.INSTANCE.mapping(builder.getAppName(), object);
           StringWriter sw  = new StringWriter();
           event.getThrown().printStackTrace(new PrintWriter(sw));
           logMeta.setContent(sw);
       }
   ```


-- 
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


Re: [I] Pass `LogEvent` to `NoSqlConnection#insertObject` (logging-log4j2)

Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on issue #2389:
URL: https://github.com/apache/logging-log4j2/issues/2389#issuecomment-2011610932

   @tonycody,
   
   I think we hit an [XY problem](https://en.wikipedia.org/wiki/XY_problem) in this discussion. Could you explain plainly (without technical details) what are you trying to achieve?
   
   From you code example I can not guess whether:
   
   - you want to modify the JSON-y object, before it is sent to the database?
   - you want to observe the events that are logged and perform some action based on them?
   - you are not satisfied with the way a `Throwable` is converted to JSON?
   - you want to make your plugin usable by users of other NoSQL databases?
   
   Let's focus on the origin of the problem and only then choose the best technical solution.
   
   > @vy I looked at the log4j redis Appender mentioned above, and it's actually a different scenario. The implementation mentioned above is based on Appender, which also uses LogEvent, but the Appender interface provides LogEvent.
   
   To send log events to a resource you need to implement `Appender`. It is also advised to extend `AbstractManager` to prevent your appender from losing events during reconfiguration. The reset is pretty much up to you.
   
   > It's interesting to note that the NoSql extension is actually a sub-implementation of the Appender class, but it's categorised in the form of NoSql, but it doesn't include the LogEvent pass.
   > 
   > This doesn't really make sense to me!
   
   Yes, `NoSqlAppender` is an implementation of `Appender` with a **fixed** way to convert a log event into a JSON-y object.
   Similarly to `XmlLayout`, `HtmlLayout`, `JsonLayout` and `YamlLayout` (but unlike `JsonTemplateLayout`) it gives you only a limited set of configuration options regarding the formatting of the event.


-- 
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


Re: [I] Pass `LogEvent` to `NoSqlConnection#insertObject` (logging-log4j2)

Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on issue #2389:
URL: https://github.com/apache/logging-log4j2/issues/2389#issuecomment-2009826857

   I am not keen on adding new methods based on speculative functionality. Could you give a more concrete example demonstrating without a new `insertObject(LogEvent,Entity)`, it is really difficult to carry out a certain business logic, please?
   
   **Background:** @tonycody, I am very thankful that you reach out to us. You did not only create a ticket, but also submitted a PR. Much appreciated. Please understand that every single line we add to the public API becomes a liability and maintenance burden we need to carry for decades. Given that the Log4j API is already pretty bloated, we need to justify every single new thing we are about to add.


-- 
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


Re: [I] Pass `LogEvent` to `NoSqlConnection#insertObject` (logging-log4j2)

Posted by "ppkarwasz (via GitHub)" <gi...@apache.org>.
ppkarwasz commented on issue #2389:
URL: https://github.com/apache/logging-log4j2/issues/2389#issuecomment-2011689567

   > My scenario is to write the stack information resolved by the method java.lang.Throwable#lockedPrintStackTrace to the NoSql data source. Therefore, I need to get the Throwable object when implementing the log4j NoSql Provider.
   
   If I understand you correctly, you don't like the way this code serializes the `Throwable`:
   
   https://github.com/apache/logging-log4j2/blob/67479ee93c94038f43b10081f3d86b742b4ef0f5/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/nosql/NoSqlDatabaseManager.java#L217-L237
   
   and you want to do it in a different way?
   
   There is **already** a hook for that: a `NoSqlAppender` can also have a [`Layout`](https://logging.apache.org/log4j/2.x/javadoc/log4j-core/org/apache/logging/log4j/core/Layout.html) component. All you need is a layout that transforms `LogEvent` into whatever you want:
   
   ```java
   @Plugin(name = "MyNoSqlLayout", category = Node.CATEGORY)
   public class MyNoSqlLayout extends AbstractLayout<MapMessage<?, ?>> {
   
       private Map<String, ?> toMap(LogEvent event) {
           /*
            * Put your logic here.
            */
       }
   
       public MyNoSqlLayout(Configuration configuration) {
           super(configuration, null, null);
       }
   
       @Override
       public byte[] toByteArray(LogEvent event) {
           return toSerializable(event).getFormattedMessage().getBytes();
       }
   
       @Override
       public MapMessage<?, ?> toSerializable(LogEvent event) {
           return new MapMessage<>(toMap(event));
       }
   
       @Override
       public String getContentType() {
           return "text/plain";
       }
   }
   ```
   
   and add it to the `NoSql` appender:
   
   ```xml
   <NoSql name="NoSql">
       ...
       <MyNoSqlLayout/>
   </NoSql>
   ```


-- 
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


Re: [I] Pass `LogEvent` to `NoSqlConnection#insertObject` (logging-log4j2)

Posted by "tonycody (via GitHub)" <gi...@apache.org>.
tonycody commented on issue #2389:
URL: https://github.com/apache/logging-log4j2/issues/2389#issuecomment-2009739402

   > @tonycody, the request sounds legit to me. Would you mind providing a bit more context, please?
   > 
   > 1. Which Log4j version are you using?
   > 2. Which appender are you using? (If it is a custom one, what is the sink it is writing to?)
   > 3. What is the use case necessitating the `LogEvent` in `insertObject()`?
   
   I'm implementing a NoSqlConnection where the received data is written to Redis. I need the original thrown object so that I can use java.lang.Throwable#printStackTrace(java.io.PrintWriter) to get the StrackTrace and convert it to String via Stream, which makes it a lot less work for me. Although StrackTrace is included in the current HashMap, I need to do some complicated parsing into String format, whereas printStackTrace is natively supported and more consistent.


-- 
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


Re: [I] Pass `LogEvent` to `NoSqlConnection#insertObject` (logging-log4j2)

Posted by "vy (via GitHub)" <gi...@apache.org>.
vy commented on issue #2389:
URL: https://github.com/apache/logging-log4j2/issues/2389#issuecomment-2009763272

   > I'm implementing a NoSqlConnection where the received data is written to Redis. I need the original thrown object so that I can use java.lang.Throwable#printStackTrace(java.io.PrintWriter) to get the StrackTrace and convert it to String via Stream, which makes it a lot less work for me. Although StrackTrace is included in the current HashMap, I need to do some complicated parsing into String format, whereas printStackTrace is natively supported and more consistent.
   
   `NoSqlDatabaseManager#writeInternal` is implemented as follows:
   
   ```
   protected void writeInternal(final LogEvent event, final Serializable serializable) {
       if (!this.isRunning() || this.connection == null || this.connection.isClosed()) {
           throw new AppenderLoggingException(
                   "Cannot write logging event; NoSQL manager not connected to the database.");
       }
   
       final NoSqlObject<W> entity = this.connection.createObject();
       if (serializable instanceof MapMessage) {
           setFields((MapMessage<?, ?>) serializable, entity);
       } else {
           setFields(event, entity);
       }
       setAdditionalFields(entity);
       this.connection.insertObject(entity);
   }
   ```
   
   Why don't you simply use `setFields()` to capture the `Throwable` and use it in `insertObject()`?
   
   I am also curious, why did you opt for implementing your own Redis appender instead of using [log4j2-redis-appender](/vy/log4j2-redis-appender)?


-- 
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