Pyroscope: Fix backend panic when querying out of bounds (#76053)

This commit is contained in:
Andrej Ocenas 2023-10-06 15:03:20 +02:00 committed by GitHub
parent 41bcb5e07f
commit 1cbae72a9e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 52 additions and 13 deletions

View File

@ -149,6 +149,11 @@ func (c *PyroscopeClient) GetProfile(ctx context.Context, profileTypeID, labelSe
return nil, err
}
if resp.Msg.Flamegraph == nil {
// Not an error, can happen when querying data oout of range.
return nil, nil
}
levels := make([]*Level, len(resp.Msg.Flamegraph.Levels))
for i, level := range resp.Msg.Flamegraph.Levels {
levels[i] = &Level{

View File

@ -51,10 +51,21 @@ func Test_PyroscopeClient(t *testing.T) {
}
require.Equal(t, series, resp)
})
t.Run("GetProfile with empty response", func(t *testing.T) {
connectClient.SendEmptyProfileResponse = true
maxNodes := int64(-1)
resp, err := client.GetProfile(context.Background(), "memory:alloc_objects:count:space:bytes", "{}", 0, 100, &maxNodes)
require.Nil(t, err)
// Mainly ensuring this does not panic like before
require.Nil(t, resp)
connectClient.SendEmptyProfileResponse = false
})
}
type FakePyroscopeConnectClient struct {
Req any
SendEmptyProfileResponse bool
}
func (f *FakePyroscopeConnectClient) ProfileTypes(ctx context.Context, c *connect.Request[querierv1.ProfileTypesRequest]) (*connect.Response[querierv1.ProfileTypesResponse], error) {
@ -75,6 +86,9 @@ func (f *FakePyroscopeConnectClient) Series(ctx context.Context, c *connect.Requ
func (f *FakePyroscopeConnectClient) SelectMergeStacktraces(ctx context.Context, c *connect.Request[querierv1.SelectMergeStacktracesRequest]) (*connect.Response[querierv1.SelectMergeStacktracesResponse], error) {
f.Req = c
if f.SendEmptyProfileResponse {
return &connect.Response[querierv1.SelectMergeStacktracesResponse]{Msg: &querierv1.SelectMergeStacktracesResponse{}}, nil
}
return &connect.Response[querierv1.SelectMergeStacktracesResponse]{
Msg: &querierv1.SelectMergeStacktracesResponse{
Flamegraph: &querierv1.FlameGraph{

View File

@ -91,10 +91,10 @@ func (d *PyroscopeDatasource) query(ctx context.Context, pCtx backend.PluginCont
logger.Error("Error GetProfile()", "err", err)
return err
}
frame := responseToDataFrames(prof)
responseMutex.Lock()
response.Frames = append(response.Frames, frame)
responseMutex.Unlock()
var frame *data.Frame
if prof != nil {
frame = responseToDataFrames(prof)
// If query called with streaming on then return a channel
// to subscribe on a client-side and consume updates from a plugin.
@ -107,6 +107,14 @@ func (d *PyroscopeDatasource) query(ctx context.Context, pCtx backend.PluginCont
}
frame.SetMeta(&data.FrameMeta{Channel: channel.String()})
}
} else {
// We still send empty data frame to give feedback that query really run, just didn't return any data.
frame = getEmptyDataFrame()
}
responseMutex.Lock()
response.Frames = append(response.Frames, frame)
responseMutex.Unlock()
return nil
})
}
@ -273,6 +281,18 @@ func (pt *ProfileTree) String() string {
return tree.String()
}
func getEmptyDataFrame() *data.Frame {
var emptyProfileDataFrame = data.NewFrame("response")
emptyProfileDataFrame.Meta = &data.FrameMeta{PreferredVisualization: "flamegraph"}
emptyProfileDataFrame.Fields = data.Fields{
data.NewField("level", nil, []int64{}),
data.NewField("value", nil, []int64{}),
data.NewField("self", nil, []int64{}),
data.NewField("label", nil, []string{}),
}
return emptyProfileDataFrame
}
type CustomMeta struct {
ProfileTypeID string
}