[Feature Request]: Print last n frames.

[DevX86]

trafficstars

Incredible project! Such a beautiful night and day difference.

Currently, we can control the amount of lines before and after the trace line in methods such as:

tracerr.SprintSourceColor(beforeLines, afterLines)

Is would be incredibly helpful if we could also specify the amount of stack frames printed. For example:

tracerr.SprintSourceColor(beforeLines, afterLines, stackFrameLimit)

Or some similar signature, getting everything every trace down to the runtime assembly is rather cool but in the majority of cases you only want the last 3-4 stack frames in any given trace for debugging. This will reduce output in terminal and show only the level of granularity we need.

Another potential attack vector which would potentially help performance:

Stack trace causes a performance overhead, depending on a stack trace depth

This could potentially be mitigated if an environmental variable such as STACK_FRAME_LIMIT was added to control the max stack length big-endian style. Only the last n stack frames would be tracked, and as a frame is added, the earliest frame is popped off. This would reduce the stack trace depth and potentially decrease performance overhead on deep traces.

[ztrue]

Hi @DevX86, thanks for the feedback and for pointing out this problem; it makes a lot of sense to me. I was not quite sure about adding an extra parameter to functions because it can be a bit challenging to follow all those optional arguments, which is actually the issue with the initial design, not the proposal. Yet, I believe it will be beneficial for the reasons you described. Regarding the package-level frame limit, I'd prefer going with a package-level variable, similar to DefaultCap. I think it will be consistent with the existing design and easy to use. I'll try to address this in the near future.

[DevX86]

Awesome! Thank you @ztrue

[fabien-marty]

👋 just implemented something about that in this commit

@ztrue interested in a pull-request?

[ztrue]

Sorry for not getting back to this issue in time. @fabien-marty, thank you for the link! Overall the change looks right and you are welcome to make a PR. There are some minor adjustments I would consider:

• i can be reused from the loop itself, e.g. by replacing this line for _, frame := range frames { with for i, frame := range frames {, some <=, etc will be replaced with just < too.
• I believe the appendedFrames counter is redundant, because DefaultIgnoreFirstFrames is a constant, and appendedFrames can be calculated at any point as i - DefaultIgnoreFirstFrames. If we are at frame 3 (starting with 0), and if we skip 2 frames, it means 1 frame was appended so far.

I know unit tests in this project are a bit of a mess, but this kind of change would require to have unit tests as well. You can address these comments or create PR as is if you prefer (in this case I can make those adjustments when testing).

[fabien-marty]

@ztrue thanks for your review => I'm going to make a PR with requested changes

for unit tests, I totally agree with you but they don't run on my env because of hardcoded paths somewhere on your own :) maybe we should fix that... before (in another PR)

=== RUN   TestError
    error_test.go:181: cases[2].Error.StackTrace()[0].Path = "/Users/fab/src/tracerr/error_test.go"; want to has suffix "/src/github.com/ztrue/tracerr/error_test.go"
    [...]

[ztrue]

The path suffix github.com/ztrue/tracerr is the name of the package. Might be worth changing to just /tracerr/error_test.go separately from this change.

[fabien-marty]

just opened a specific issue for unit tests not passing out of the box: https://github.com/ztrue/tracerr/issues/12

[DevX86]

This is awesome guys!