support generating a flamegraph from a list of chunk IDs #476
Conversation
when a chunk has a start and stop ts specified, we'll slice the callTree in order to only use data within that time-range to generate the flamegraph.
…grtaph-generation-from-chunks
accept a list of intervals for each chunk.
internal/flamegraph/flamegraph.go
Outdated
| if errors.Is(res.Err, context.DeadlineExceeded) { | ||
| return speedscope.Output{}, nil | ||
| } |
There was a problem hiding this comment.
Thinking out loud here but if the result of 1 chunk is a deadline exceeded, and the rest succeeds, should we still return the flamegraph with the successful results anyways?
There was a problem hiding this comment.
That's a good question.
For the flamechart endpoint, where we want to return a merged sequence of chunks as a unified profile, even if 1 chunk fails for whatever reason (not only timeout), we return an error as the final merged chunk wouldn't make sense with gaps in it.
For the flamegraph, being it an aggregation, I agree with you that this should not be a big issue and partial flamegraph is still better than no flamegraph.
The reason I'm returning the error with an empty flamegraph here though, is that the context (ctx) we pass to the goroutine is the one from the parent request, so if that operation failed due to a context.DeadlineExceeded, we've likely exceeded the parent timeout so the whole request timed-out at that point and returning a partial one won't make a difference at this point.
internal/flamegraph/flamegraph.go
Outdated
| } | ||
|
|
||
| sp := toSpeedscope(flamegraphTree, 4, projectID) | ||
| hub.Scope().SetTag("processed_chunks", strconv.Itoa(countChunksAggregated)) |
There was a problem hiding this comment.
Could be helpful to set the number of chunks requested as well. And this could be more useful as a metric/measurement (whatever is possible) than a tag because sending this as a string is not very useful.
There was a problem hiding this comment.
Yes, I'll add the requested_chunk as well to the tags in the endpoint I'll add to handle the request (similarly to what we do for the other flamegraph endpoint).
And this could be more useful as a metric/measurement (whatever is possible)
Yes. That's better handled on the Sentry side of the request though as we have the utilities in the python SDK to easily create metrics.
Each interval that we want to slice our chunk on, might belong to a different span that was created on a different thread. This takes care of that. Moreover, in this way we can optimize and re-use the same chunk instead of downloading the same chunk multiple times.
return the error instead of nil
No description provided.