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

Update StackTraces.ScrubAndTruncate to handle zero max depth #523

Merged
merged 2 commits into from
Apr 8, 2021

Conversation

nr-ahemsath
Copy link
Member

@nr-ahemsath nr-ahemsath commented Apr 7, 2021

Description

This fixes #522 by adjusting the logic of the StackTraces.ScrubAndTruncate method to return an empty list when the maxDepth parameter is set to 0.

Testing

Added one unit test to cover this case. I also manually tested a test app that threw an exception from a stack depth of 3 and verified that no stack trace lines are sent in the error data when maxStackTraceLines is set to 0 in the agent config file.

Changelog

Changelog is updated.

@nr-ahemsath nr-ahemsath requested a review from tehbio April 7, 2021 20:47
@nr-ahemsath nr-ahemsath added the bug Something isn't working label Apr 7, 2021
@nr-ahemsath nr-ahemsath added the on-hold On hold pending more information label Apr 7, 2021
@nr-ahemsath nr-ahemsath marked this pull request as draft April 7, 2021 22:39
@nr-ahemsath
Copy link
Member Author

Converted to draft while waiting for answers to some questions. There is a question of whether a config value of "0" should actually mean "0", or if it should mean "infinity". I checked the agent specs for error traces and I didn't find an answer either way. I'm working on getting input from other agent teams to see what they do.

@nr-ahemsath nr-ahemsath marked this pull request as ready for review April 8, 2021 15:12
@nr-ahemsath
Copy link
Member Author

No conclusive feedback either way; all our integration tests pass for me locally with this change, and it also seems unlikely that the backend would like the agent to send them an "infinite" amount of stack frames in any case. Going ahead with this change.

@nr-ahemsath nr-ahemsath merged commit 2ba99ea into main Apr 8, 2021
@nr-ahemsath nr-ahemsath deleted the fix-stack-trace-truncate-logic branch April 8, 2021 15:14
@nr-ahemsath nr-ahemsath removed the on-hold On hold pending more information label Jul 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

maxStackTraceLines="0" is not honored
2 participants