search

gemnasium / logrus-graylog-hook

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

Use logrus ReportCaller to get file, line and function #39

Closed psampaz closed 5 years ago

psampaz commented 5 years ago

This pull request is related to issue #38

Logrus now can provide the caller file, line and function is the reportCaller field is set to true.

This PR is to use Logrus existing functionality instead of the hook's similar implementation.

Gelf format consider File and Line fields deprecated and suggest to use additional fields instead. In order to minimize side effects File and Line are still populated with the correct values if reportCaller field is set to true.

In short the changes are the following:

A. When reportCaller is set to true then three additional fields _file, _method and _line are added. File and Line field use the same values as _file and _line

B. When reportCaller is set to false then three additional fields _file, _method and _line are NOT added. File and Line field use the zero values of their corresponding types, . "" and 0 respectively.

Tests to cover the above two senarios are included

psampaz commented 5 years ago

@gravis any tips why TestStackTracer fail only in Go:tip https://travis-ci.org/gemnasium/logrus-graylog-hook/jobs/470174952 ?

Something I can look in more detail and make the test pass?

I am not up to date with the developent version of Go.

psampaz commented 5 years ago

The problem is that in tip the stacktrace include a new line that is not covered by the existing regex in tests (https://github.com/gemnasium/logrus-graylog-hook/blob/master/graylog_hook_test.go#L285):

gopkg.in/gemnasium/logrus-graylog-hook%2ev2.TestStackTracer /home/travis/gopath/src/github.com/pkg/errors/errors.go:105 runtime.skipPleaseUseCallersFrames /home/travis/.gimme/versions/go/src/runtime/asm.s:40 testing.tRunner /home/travis/.gimme/versions/go/src/testing/testing.go:862 runtime.goexit /home/travis/.gimme/versions/go/src/runtime/asm_amd64.s:1337

This seems like a really fragile test. I can try to put an optional regex to match just this extra line but it does not really make sense.

Can someone of the approvers advise on how to handle breaking tests on Go:tip?

gravis commented 5 years ago

@psampaz I don't see a simple solution fo that. One option could be to tweak the regexp. Another is to generate the expected trace (using GOPATH, etc.), but I'm bit afraid of edge cases. A template won't work, as the lines are changing between tip and current go versions :(

psampaz commented 5 years ago

ok, I will try to tweak the regex and make the tip pass.