Ability to add custom fields to GelfMessage, computed from ILoggingEvent about logback-gelf HOT 20 CLOSED

Thanks for your ideas! I think, a more generic approach to map the additional fields (as you described) would be very good and makes it easier extensible. Would be great if you could provide a PR.

from logback-gelf.

I added a PR together with the issue, you didn't notice it?

from logback-gelf.

I noticed, but more things needs to be done, like test, documentation, examples, ...

from logback-gelf.

True, I did not want to invest too much time without first getting a confirmation from you that the direction is acceptable.

from logback-gelf.

Is the direction acceptable?

from logback-gelf.

Sorry for the misunderstanding. You wrote:

If you want to explore this idea further, you could even refactor GelfEncoder and replace all include* properties with specific built-in "enrichment" processors. This would simplify that class A LOT.

With my initial answer I tried to say that I'd like to go that direction. Unfortunately I currently do not have the time to refactor the GelfEncoder by myself so I'd appreciate if you could provide a PR for this.

from logback-gelf.

Great! I plan to do 2 things during the upcoming weeks:

• add unit tests to the existing PR
• add another PR that would refactor the GelfEncoder class

from logback-gelf.

I think it would be the best if you focus on the PR for refactoring the GelfEncoder class right away. I don't want to merge the 'small' solution from the first PR.

from logback-gelf.

I now updated the PR and refactored GelfEncoder as we agreed.
There are some CheckStyle errors:

Error: eckstyle] [ERROR] /home/runner/work/logback-gelf/logback-gelf/src/main/java/de/siegmar/logbackgelf/GelfEncoder.java:43:1: Class Data Abstraction Coupling is 8 (max allowed is 7) classes [CallerDataFieldMapper, GelfFieldHelper, GelfMessage, MarkerFieldMapper, MdcDataFieldMapper, PatternLayout, RootExceptionDataFieldMapper, SimpleFieldMapper]. [ClassDataAbstractionCoupling]

Error: eckstyle] [ERROR] /home/runner/work/logback-gelf/logback-gelf/src/main/java/de/siegmar/logbackgelf/GelfEncoder.java:347: Line is longer than 100 characters (found 102). [LineLength]
> Task :checkstyleMain
Error: eckstyle] [ERROR] /home/runner/work/logback-gelf/logback-gelf/src/main/java/de/siegmar/logbackgelf/GelfFieldHelper.java:81: Line is longer than 100 characters (found 107). [LineLength]
Error: eckstyle] [ERROR] /home/runner/work/logback-gelf/logback-gelf/src/main/java/de/siegmar/logbackgelf/GelfFieldHelper.java:84: Line is longer than 100 characters (found 109). [LineLength]
Error: eckstyle] [ERROR] /home/runner/work/logback-gelf/logback-gelf/src/main/java/de/siegmar/logbackgelf/GelfFieldMapper.java:27: Line is longer than 100 characters (found 111). [LineLength]
Error: eckstyle] [ERROR] /home/runner/work/logback-gelf/logback-gelf/src/main/java/de/siegmar/logbackgelf/mappers/MdcDataFieldMapper.java:46: Line is longer than 100 characters (found 107). [LineLength]

I find these to be rather restrictive and would suggest to lift these:

• Either remove Class Data Abstraction Coupling check or increase it to at least 10.
• I would suggest to increase line length limit to allow 120 characters on single line. Monitors are quite large nowadays.

from logback-gelf.

OK, could you please:

• Add @SuppressWarnings("checkstyle:classdataabstractioncouling") to the GelfEncoder class in order to let the pipeline complete
• Change the line length limit to 120 in checkstyle.xml
• Merge the master (I added code codecov code coverage analysis)

from logback-gelf.

Done

from logback-gelf.

Thank you. Give me a few days (probably until end of week) to review it.

from logback-gelf.

Thanks for your work. It really looks great! I'll play a little with it and will perform some small changes. I'll get back to you in order to check if those changes are breaking anything you had in mind.

from logback-gelf.

You now may want to take a look on c372fe8

from logback-gelf.

Two small improvements to your changes, see pull request #57.

from logback-gelf.

Thanks again!

from logback-gelf.

Any plans for a release?

from logback-gelf.

See https://github.com/osiegmar/logback-gelf/milestone/1 ;-)

from logback-gelf.

Still no release :-( Any plans?

from logback-gelf.

Yep. I also makes me a little sad. In case you have some spare time left, you could help on #59 to finish the release.

from logback-gelf.