search

gemnasium / logrus-graylog-hook

Graylog hook for logrus
MIT License
87 stars 63 forks source link

When log.SetReportCaller(true) file and func fields are not visible #38

Closed psampaz closed 5 years ago

psampaz commented 5 years ago

When I enable the report caller in logrus by setting log.SetReportCaller(true), then the fields file and func are visible when in stdout.

In Graylog these fields are not visible at all.

Is this a bug or normal behavior?

gravis commented 5 years ago

Hi, thanks for raising this. The Entry in Graylog has been updated to contain a Caller field, whereas we don't have that field in this plugin. We're accepting Pull Requests if you want to contribute and fix this behavior. It should be trivial to add and test :)

psampaz commented 5 years ago

Hi @gravis

Caller is a runtime.Frame (https://golang.org/pkg/runtime/#Frame) and includes file, line and function fields.

Should these be added as additional extra fields (_file, _line, _function) or replace GELF's file and line field and pass only function as extra field?

I am asking because according to GELF's format both file and line fields are deprecated and the guideline is to use additional fields (http://docs.graylog.org/en/stable/pages/gelf.html)

Please advise and I will proceed with a MR

gravis commented 5 years ago

haaa, very good point. I think it's a good candidate for a breaking change in the plugin. We could get rid of duplicate code, and follow both Logrus and Graylog convention. Hence, if we bump this plugin to 3.0, I'm fine using _file, _line, _function. That will be the occasion to start the transition to go Modules as well. Thanks

gravis commented 5 years ago

Could you test with the v3 version of this plugin? I don't have a graylog here to test in real condition. Thanks (feel free to open another issue if you spot new side effects)

psampaz commented 5 years ago

yes @gravis, I will try to do this tomorrow. I will update you with the results.