Uploaded image for project: 'Samza'
  1. Samza
  2. SAMZA-597

Support a JSON encoder for Log4J StreamAppender

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.9.0
    • Fix Version/s: 0.9.0
    • Component/s: log4j
    • Labels:
      None

      Description

      We are trying to hook a Samza job up to ELK. During this exercise, I found that the only format available is a String-encoded LoggingEvent. Logstash has a well-defined LoggingEvent JSON encoding. It would be nice if Samza supported this.

      1. SAMZA-597-1.patch
        33 kB
        Chris Riccomini
      2. SAMZA-597-0.patch
        29 kB
        Chris Riccomini

        Activity

        Hide
        criccomini Chris Riccomini added a comment -

        Thanks, all! Merged and committed.

        Show
        criccomini Chris Riccomini added a comment - Thanks, all! Merged and committed.
        Hide
        closeuris Yan Fang added a comment -

        look good. +1

        Show
        closeuris Yan Fang added a comment - look good. +1
        Hide
        nickpan47 Yi Pan (Data Infrastructure) added a comment -

        +1. LGTM.

        Show
        nickpan47 Yi Pan (Data Infrastructure) added a comment - +1. LGTM.
        Hide
        criccomini Chris Riccomini added a comment -

        Attaching updated patch with Yi Pan (Data Infrastructure)'s feedback.

        Show
        criccomini Chris Riccomini added a comment - Attaching updated patch with Yi Pan (Data Infrastructure) 's feedback.
        Hide
        criccomini Chris Riccomini added a comment -

        Attaching a patch. RB at:

        https://reviews.apache.org/r/32006

        Changes:

        1. Wrote a LoggingEventJsonSerde, and LoggingEventJsonSerdeFactory.
        2. Added a config to define whether the JSON encoding should include LocationInfo. Defaulted to false.
        3. Updated configuration-table.html docs, and logging.md docs.
        4. Updated tests to make sure everything still works.
        5. Misc cleanup in Log4jSystemConfig to shrink lines of code.
        6. Changed default serializer from string to JSON. This matches the default CheckpointManager serialization format, and also makes it easier for Samza to integrate with ELK. This format is also less lossy than the LoggingEventStringSerde.
        7. Did not implement JSON-to-LoggingEvent decoding. Wasn't required for ELK integration.
        8. Throw an exception if the containerName SystemProperty is empty.

        Also ran with hello-samza, and an internal job. Fully integrated the messages with ELK, and it worked.

        Show
        criccomini Chris Riccomini added a comment - Attaching a patch. RB at: https://reviews.apache.org/r/32006 Changes: Wrote a LoggingEventJsonSerde, and LoggingEventJsonSerdeFactory. Added a config to define whether the JSON encoding should include LocationInfo . Defaulted to false. Updated configuration-table.html docs, and logging.md docs. Updated tests to make sure everything still works. Misc cleanup in Log4jSystemConfig to shrink lines of code. Changed default serializer from string to JSON. This matches the default CheckpointManager serialization format, and also makes it easier for Samza to integrate with ELK. This format is also less lossy than the LoggingEventStringSerde. Did not implement JSON-to-LoggingEvent decoding. Wasn't required for ELK integration. Throw an exception if the containerName SystemProperty is empty. Also ran with hello-samza, and an internal job. Fully integrated the messages with ELK, and it worked.

          People

          • Assignee:
            criccomini Chris Riccomini
            Reporter:
            criccomini Chris Riccomini
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development