Repository navigation
Read sourcemap content and fill context lines - #972
Conversation
| "github.com/elastic/beats/libbeat/logp" | ||
| ) | ||
|
|
||
| const sourcemapContentSnippetSize = 5 |
There was a problem hiding this comment.
This could have a huge impact on the used storage so I suggest to make this configurable.
There was a problem hiding this comment.
you think so? this is in errors - not spans... and only for frontend with sources in uploaded sourcemaps
i didn't want to bloat more the configuration space, but i can make it configurable if you prefer
There was a problem hiding this comment.
Sourcemapping is generally enabled for frontend endpoints, so it will also be enabled for transactions, not only for errors.
If we don't add the config option, it will be hard to give advice to customers in case they want to use sourcemapping but reduce storage.
|
|
||
| func subSlice(from, to int, content []string) []string { | ||
| if from > len(content) { | ||
| return content |
There was a problem hiding this comment.
If I read correct, then in this case we would store the whole content. I suggest to return an empty array and to log an error.
There was a problem hiding this comment.
this is for when source content is not available, will rewrite to if len(content) == 0 to make it clear; sorry for that.
not sure about logging an error, sourcemap content is optional (as per spec); i think some tools include it by default and others don't, users can choose.
you still think should be logged?
There was a problem hiding this comment.
Oh ok. I agree that there is no need for logging when you check if len(content) == 0 { return content }, as this reflects something different than checking that from is not an overflow of the content array.
reads content from sourcemaps if available, and adds a code snippet to the stacktrace fixes elastic#457
|
Awesome to have this in! We should probably add a changelog entry? |
fixes #457