Log4j 2
  1. Log4j 2
  2. LOG4J2-355

Add support for multiple SD-ELEMENTs in a RFC 5424 syslog message

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0-beta9
    • Fix Version/s: 2.0-beta9
    • Component/s: Core
    • Labels:
      None

      Description

      The patch contains the modifications to support the feature and I also refactored some code in the RFC5424Layout.java file.

      1. add_multime_SD_ELEMENT_support.diff
        24 kB
        Tibor Benke
      2. eventPrefix_behaviour_demo_testcase.patch
        2 kB
        Tibor Benke
      3. LOG4J2-355_v2.patch
        28 kB
        Tibor Benke
      4. LOG4J2-355_v3.patch
        29 kB
        Tibor Benke
      5. LOG4J2-355_v4.patch
        30 kB
        Tibor Benke

        Activity

        Hide
        Tibor Benke added a comment -

        I added a "discardIfAllFieldsAreEmpty" boolean attribute to LoggerFields, so if all fields are empty during the evaluation in the RFC5424Layout, one can control that this SD-ELEMENT should be discarded or not.

        Show
        Tibor Benke added a comment - I added a "discardIfAllFieldsAreEmpty" boolean attribute to LoggerFields, so if all fields are empty during the evaluation in the RFC5424Layout, one can control that this SD-ELEMENT should be discarded or not.
        Hide
        Tibor Benke added a comment -

        What do you think about this? Please, feel free to share your comments and thoughts about it.

        Show
        Tibor Benke added a comment - What do you think about this? Please, feel free to share your comments and thoughts about it.
        Hide
        Gary Gregory added a comment -

        Hi Tibor,

        Could you please recreate your patch on top of trunk? I am having trouble applying it even after replacing "core/src" with "log4j-core/src".

        Thank you,
        Gary

        Show
        Gary Gregory added a comment - Hi Tibor, Could you please recreate your patch on top of trunk? I am having trouble applying it even after replacing "core/src" with "log4j-core/src". Thank you, Gary
        Hide
        Tibor Benke added a comment -

        I updated the patch (LOG4J2-355_v3.patch) to the current trunk version.

        Show
        Tibor Benke added a comment - I updated the patch ( LOG4J2-355 _v3.patch) to the current trunk version.
        Hide
        Tibor Benke added a comment -

        Unfortunately, I found a bug so please, don't apply until I recreate a new patch.

        Show
        Tibor Benke added a comment - Unfortunately, I found a bug so please, don't apply until I recreate a new patch.
        Hide
        Tibor Benke added a comment -

        I solved the bug and attached the new (and hopefully last) patch (LOG4J2-355_v4.patch).

        Show
        Tibor Benke added a comment - I solved the bug and attached the new (and hopefully last) patch ( LOG4J2-355 _v4.patch).
        Hide
        Gary Gregory added a comment -

        Hello Tibor,

        I've applied the patch locally, made changes and committed.

        Since you've been in there, can you please comment on:

        • The field evenPrefix is not used, should it or can it be removed?
        • The method getId(StructuredDataId) is not used, should it or can it be removed?

        Thank you!

        Gary

        Show
        Gary Gregory added a comment - Hello Tibor, I've applied the patch locally, made changes and committed. Since you've been in there, can you please comment on: The field evenPrefix is not used, should it or can it be removed? The method getId(StructuredDataId) is not used, should it or can it be removed? Thank you! Gary
        Hide
        Tibor Benke added a comment -

        Hi Gary,

        I think, we won't need anymore the getId(StructuredDataId) method, so you can remove it. It's functionality partially implemented in the LoggerFields class.
        On the other hand, as far as I can see we used the eventPrefix to mark all key-value pairs in a StructuredDataMessage. I forgot to reimplement this during the partial refactor, but after all, I think there is a better solution. Now, you can create a new SD-Element from a StructuredDataMessage, which is IMHO a more elegant way to "prefix" our key-value pairs.

        But, it's easy to use the eventPrefix again. What do you think about it?

        P.S: for demonstrating the behaviour what I described above, I'm going to upload a new testcase to RFC5424LayoutTest.java.

        Best wishes,
        Tibor

        Show
        Tibor Benke added a comment - Hi Gary, I think, we won't need anymore the getId(StructuredDataId) method, so you can remove it. It's functionality partially implemented in the LoggerFields class. On the other hand, as far as I can see we used the eventPrefix to mark all key-value pairs in a StructuredDataMessage. I forgot to reimplement this during the partial refactor, but after all, I think there is a better solution. Now, you can create a new SD-Element from a StructuredDataMessage, which is IMHO a more elegant way to "prefix" our key-value pairs. But, it's easy to use the eventPrefix again. What do you think about it? P.S: for demonstrating the behaviour what I described above, I'm going to upload a new testcase to RFC5424LayoutTest.java. Best wishes, Tibor
        Hide
        Gary Gregory added a comment -

        Hi Tibor,

        I think we should provide the best design and implementation for 2.0, whatever that ends up being WRT eventPrefix. I'd like to let you propose a patch with what you think is the best way to go as you seem to have some good knowledge of this feature.

        Thank you,
        Gary

        Show
        Gary Gregory added a comment - Hi Tibor, I think we should provide the best design and implementation for 2.0, whatever that ends up being WRT eventPrefix. I'd like to let you propose a patch with what you think is the best way to go as you seem to have some good knowledge of this feature. Thank you, Gary
        Hide
        Ralph Goers added a comment -

        I haven't had a chance to review this. I need to see what has changed to make sure everything is still ok.

        Show
        Ralph Goers added a comment - I haven't had a chance to review this. I need to see what has changed to make sure everything is still ok.
        Hide
        Tibor Benke added a comment -

        The eventPrefix was introduced by Ralph in this commit:

        http://mail-archives.apache.org/mod_mbox/logging-commits/201304.mbox/%3C20130418224222.4B0A62388A29@eris.apache.org%3E

        Ralph, do you think it is still needed? In that case I'm going to make a patch.
        And I'm going to write some docs about the LoggerFields in the RFC5424Layout's site documentation after we made a decision.

        Show
        Tibor Benke added a comment - The eventPrefix was introduced by Ralph in this commit: http://mail-archives.apache.org/mod_mbox/logging-commits/201304.mbox/%3C20130418224222.4B0A62388A29@eris.apache.org%3E Ralph, do you think it is still needed? In that case I'm going to make a patch. And I'm going to write some docs about the LoggerFields in the RFC5424Layout's site documentation after we made a decision.

          People

          • Assignee:
            Unassigned
            Reporter:
            Tibor Benke
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development