Skip to content
This repository has been archived by the owner on Jun 29, 2023. It is now read-only.

Switch off additional fields while using Logback #144

Closed
burningalchemist opened this issue May 16, 2018 · 13 comments
Closed

Switch off additional fields while using Logback #144

burningalchemist opened this issue May 16, 2018 · 13 comments
Labels
type: enhancement A general enhancement

Comments

@burningalchemist
Copy link

burningalchemist commented May 16, 2018

Hi!

Could you please advise how I can switch off all the additional fields while using logback, like these:

SourceClassName
SourceSimpleClassName
SourceMethodName
SourceLineNumber

Currently, when default-logstash-fields.properties doesn't exist I observe all the additional fields. So far I would like to keep only default fields (as in the documentation) like:

host
short_message
full_message
timestamp (Java millis/1000)
level (syslog severity)
facility

I have found a workaround, when I provide default-logstash-fields.properties with the content like:
""=Time, then I see only default values in my logging solution. Still, it looks ugly and I think there should be a better way.

Kind regards,
Sergei

@mp911de
Copy link
Owner

mp911de commented May 16, 2018

default-logstash-fields.properties allows customization of fields but it does not allow switching off all fields. It must contain at least one field.

Care to elaborate why you want to disable all auxiliary fields? That's typically a task for logstash filters.

@burningalchemist
Copy link
Author

Got it, thank you!

Generally, to keep the log output more uniform with the other applications without additional adjustments on Logstash side. They are also described in the documentation as additional fields, so my expectation was that by default I only send default fields.

Currently, my team is using log4j2 for apps, and recently another app was developed, which is using logback. So the idea was to achieve the same message layout without adding new fields and touching filters.

@mp911de
Copy link
Owner

mp911de commented May 17, 2018

This library wasn't really developed for this use but rather to submit as much data as possible.
However, feel free to fork this library and tailor it to your needs.

I'd keep this ticket open to see how common this use case is and wait for votes.

@manasvigupta
Copy link

manasvigupta commented May 23, 2018

Hello,

First, I would like to thank you for creating this useful library.

Coming to this issue - I think few attributes should be optional & turned off by default. These are listed below and they have common characteristic - performance impact while logging using log4j. Essentially, all these attributes that are related to caller location logging.

SourceClassName
SourceSimpleClassName
SourceMethodName
SourceLineNumber

There is performance cost while logging this info since log4j creates new Throwable() for each log statement & parses it.

I have blogged more about this issue here -> https://medium.com/@manasvi/do-not-enable-caller-location-logging-with-log4j-3c90f9aaabdb

Either,

  1. these attributes should be optional and turned off by default, or,
  2. there should be a way to turn off these attributes.

Let me know your thoughts.

Thanks
Manasvi

@manasvigupta
Copy link

Also, my suggestion is to prefer option # 1 (from previous comment) i.e. turn off caller location attributes by default.

Just for comparison - log4j2, which support gelf layout also does not log caller location attributes by default (i don't think it provides any option to enable it also).

@mp911de
Copy link
Owner

mp911de commented May 24, 2018

Thanks a lot. Thinking in the way of including/excluding caller location details is a reasonable approach that does not touch filtering of fields. Adding this configuration switch is an easy exercise as we need to add configuration flags to the appender/formatters we're providing.

Location details would stay included by default to not break the current behavior. Users that want to disable location details could set e.g. IncludeLocation=false.

@mp911de mp911de added type: enhancement A general enhancement and removed waiting for votes labels May 24, 2018
@manasvigupta
Copy link

Ok that works too. Thanks!

@manasvigupta
Copy link

I am going to take stab at this today. I haven't looked at code yet, but, I am hoping its a small change.

@manasvigupta
Copy link

Ok, so, I have fix ready & tested with Log4J.

I will raise a pull request for your review by this weekend or early next week.

@mp911de mp911de added this to the logstash-gelf 1.12.0 milestone Jun 12, 2018
mp911de added a commit that referenced this issue Jun 12, 2018
logstash-gelf now allows disabling source location fields to reduce the amount of fields that are sent with the GELF messages and to reduce cost of log event submission when source details are not needed.
@mp911de
Copy link
Owner

mp911de commented Jun 12, 2018

That's in place now.

@mp911de mp911de closed this as completed Jun 12, 2018
@manasvigupta
Copy link

Nice. 👍

You beat me to it.

How long does it for 1.12.0 version (with these changes) to reflect in maven repository?

@manasvigupta
Copy link

Hi,

I am seeing this warning when i use "includeLocation" attribute as param.

log4j:WARN No such property [includeLocation] in biz.paluch.logging.gelf.log4j.GelfLayout.

It seems that setter is missing in GelfLayout.java.

Thanks
Manasvi

@mp911de
Copy link
Owner

mp911de commented Jun 25, 2018

@manasvigupta Can you file a new bug report?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants