Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add skipPackageNameForCaller #898

Closed
wants to merge 7 commits into from
Closed

add skipPackageNameForCaller #898

wants to merge 7 commits into from

Conversation

tossp
Copy link

@tossp tossp commented Jan 27, 2019

It's just an implementation. It can only be used, but it's not perfect. I hope there's a better solution.😋

ex:

logrus.AddSkipPackageFromStackTrace("someprogram/package/subpackage")
logrus.AddSkipPackageFromStackTrace("tsl/core/log")
logrus.AddSkipPackageFromStackTrace("github.com/go-xorm/xorm")

#887

ex:
```golang
logrus.SetSkipPackageNameForCaller("tsl/core/log")
logrus.SetSkipPackageNameForCaller("github.com/go-xorm/xorm")
```
@eightnoteight
Copy link

this should help in resolving #867 too

entry.go Outdated Show resolved Hide resolved
@@ -40,6 +40,11 @@ func init() {
minimumCallerDepth = 1
}

// Set the global qualified package name.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the examples here as well.

entry.go Outdated
} else {
minimumCallerDepth = knownLogrusFrames
}
skipPackageNameForCaller[getPackageName(runtime.FuncForPC(pcs[0]).Name())] = true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’d prefer having a local logrusPackage variable, this is quite hard to read.

entry.go Outdated
// now that we have the cache, we can skip a minimum count of known-logrus functions
// XXX this is dubious, the number of frames may vary store an entry in a logger interface
minimumCallerDepth = knownLogrusFrames
if len(skipPackageNameForCaller) != 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain what is going on here?
I have the impression minimumCallerDepth = knownLogrusFrames should remain the same.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @lavoiesl @tossp ,
won't this be unused since we are already including the logrus package in the ignore/skip map?

Co-Authored-By: tossp <code@tossp.com>
@dgsb
Copy link
Collaborator

dgsb commented Mar 23, 2019

I'm not such fan of the interface, I wonder if it would not be more practical for the user to provide a callback function returning a boolean which would allow to implement any kind of logic.
Not sure what would be the parameter of the callback function though. Maybe just the current frame and and its index.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants