search

Moocar / logback-gelf

Logback plugin to send GELF messages to graylog2 server
Apache License 2.0
147 stars 59 forks source link

Inconsistent field type conversion #61

Open jfachal opened 9 years ago

jfachal commented 9 years ago

Hi,

In my humble opinion, there is an inconsistent within the treatment of the field type conversion.

The doc says: "You can configure a specific field to be converted to a numeric type. Key is the additional field key (and should thus begin with an underscore), value is the type to convert to. Currently supported types are int, long, float and double."

But this is only true when you set the flag "includeFullMDC" to true.

If you don't set the explicit flag, and you create explicit mapping for additional fields, then the field type conversion is in the form:

requestId:_request_id requestId:long

I think that this could be easily changed with a minimun change in the file GelfConverter.java to unified criteria: Proposed change (line 202) Replace: map.put(additionalFields.get(key), convertFieldType(field, key)); With: map.put(additionalFields.get(key), convertFieldType(field, additionalFields.get(key)));

Another aproach could be:

Replace (line 193 and line 195) map.put(additionalFields.get(e.getKey()), convertFieldType(e.getValue(), additionalFields.get(e.getKey()))); map.put("" + e.getKey(), convertFieldType(e.getValue(), "" + e.getKey()));

With map.put(additionalFields.get(e.getKey()), convertFieldType(e.getValue(), e.getKey())); map.put("_" + e.getKey(), convertFieldType(e.getValue(), e.getKey()));

But this impies also changing the main doc page.

exet3150 commented 9 years ago

Hi,

by the flag you meant true

cheers, P.