Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/pyroscope/help-all.txt.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,6 @@ Usage of ./pyroscope:
-validation.max-profile-stacktrace-samples int
Maximum number of samples in a profile. 0 to disable. (default 16000)
-validation.max-profile-symbol-value-length int
Maximum length of a profile symbol value (labels, function names and filenames, etc...). Profiles are not rejected instead symbol values are truncated. 0 to disable. (default 1024)
Maximum length of a profile symbol value (labels, function names and filenames, etc...). Profiles are not rejected instead symbol values are truncated. 0 to disable. (default 65535)
-version
Show the version of pyroscope and exit
2 changes: 1 addition & 1 deletion cmd/pyroscope/help.txt.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ Usage of ./pyroscope:
-validation.max-profile-stacktrace-samples int
Maximum number of samples in a profile. 0 to disable. (default 16000)
-validation.max-profile-symbol-value-length int
Maximum length of a profile symbol value (labels, function names and filenames, etc...). Profiles are not rejected instead symbol values are truncated. 0 to disable. (default 1024)
Maximum length of a profile symbol value (labels, function names and filenames, etc...). Profiles are not rejected instead symbol values are truncated. 0 to disable. (default 65535)
-version
Show the version of pyroscope and exit

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ limits:
# filenames, etc...). Profiles are not rejected instead symbol values are
# truncated. 0 to disable.
# CLI flag: -validation.max-profile-symbol-value-length
[max_profile_symbol_value_length: <int> | default = 1024]
[max_profile_symbol_value_length: <int> | default = 65535]

# The tenant's shard size used by shuffle-sharding. Must be set both on
# ingesters and distributors. 0 disables shuffle sharding.
Expand Down
4 changes: 3 additions & 1 deletion pkg/frontend/frontend_select_merge_stacktraces.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ func (f *Frontend) SelectMergeStacktraces(ctx context.Context,
return nil, err
}

t := m.Tree()
t.FormatNodeNames(phlaremodel.DropGoTypeParameters)
return connect.NewResponse(&querierv1.SelectMergeStacktracesResponse{
Flamegraph: m.FlameGraph(c.Msg.GetMaxNodes()),
Flamegraph: phlaremodel.NewFlameGraph(t, c.Msg.GetMaxNodes()),
}), nil
}
24 changes: 24 additions & 0 deletions pkg/model/fmt.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package model

import (
"regexp"
"strings"
)

var goStructTypeParameterRegex = regexp.MustCompile(`\[go\.shape\..*\]`)

func DropGoTypeParameters(input string) string {
matchesIndices := goStructTypeParameterRegex.FindAllStringIndex(input, -1)
if len(matchesIndices) == 0 {
return input
}
var modified strings.Builder
prevEnd := 0
for _, indices := range matchesIndices {
start, end := indices[0], indices[1]
modified.WriteString(input[prevEnd:start] + "[...]")
prevEnd = end
}
modified.WriteString(input[prevEnd:])
return modified.String()
}
Comment on lines +10 to +24
Copy link
Collaborator Author

@kolesnikovae kolesnikovae Aug 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that we don't know if it is a Go profile / node, therefore the function is applied to all nodes. ReplaceAllString would look better here but it allocates a new string.

I'm also not sure if there might be more than one group of type parameters.

30 changes: 30 additions & 0 deletions pkg/model/fmt_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package model

import (
"bufio"
"os"
"testing"

"github.com/stretchr/testify/require"
)

func Test_DropGoTypeParameters(t *testing.T) {
ef, err := os.Open("testdata/go_type_parameters.expected.txt")
require.NoError(t, err)
defer ef.Close()

in, err := os.Open("testdata/go_type_parameters.txt")
require.NoError(t, err)
defer in.Close()

input := bufio.NewScanner(in)
expected := bufio.NewScanner(ef)
for input.Scan() {
expected.Scan()
require.Equal(t, expected.Text(), DropGoTypeParameters(input.Text()))
}

require.NoError(t, input.Err())
require.NoError(t, expected.Err())
require.False(t, expected.Scan())
}
11 changes: 11 additions & 0 deletions pkg/model/testdata/go_type_parameters.expected.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
github.com/bufbuild/connect-go.(*Client[...]).CallUnary
github.com/bufbuild/connect-go.(*Request[...]).setRequestMethod
github.com/bufbuild/connect-go.(*Response[...]).Header
github.com/bufbuild/connect-go.NewClient[...].func2
github.com/bufbuild/connect-go.NewRequest[...]
github.com/bufbuild/connect-go.NewResponse[...]
github.com/bufbuild/connect-go.NewUnaryHandler[...].func2
github.com/grafana/pyroscope/pkg/util/loser.(*Tree[...]).Next
runtime/internal/atomic.(*Pointer[...]).Load
sync/atomic.(*Pointer[...]).Store
github.com/grafana/pyroscope/pkg/phlaredb/symdb.(*deduplicatingSlice[...]).ingest
11 changes: 11 additions & 0 deletions pkg/model/testdata/go_type_parameters.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
github.com/bufbuild/connect-go.(*Client[go.shape.struct { github.com/grafana/pyroscope/api/gen/proto/go/push/v1.state google.golang.org/protobuf/internal/impl.MessageState; github.com/grafana/pyroscope/api/gen/proto/go/push/v1.sizeCache int32; github.com/grafana/pyroscope/api/gen/proto/go/push/v1.unknownFields []uint8; Series []*github.com/grafana/pyroscope/api/gen/proto/go/push/v1.RawProfileSeries \"protobuf:\\\"bytes,1,rep,name=series,proto3\\\" json:\\\"series,omitempty\\\"\" },go.shape.struct { github.com/grafana/pyroscope/api/gen/proto/go/push/v1.state google.golang.org/protobuf/internal/impl.MessageState; github.com/grafana/pyroscope/api/gen/proto/go/push/v1.sizeCache int32; github.com/grafana/pyroscope/api/gen/proto/go/push/v1.unknownFields []uint8 }]).CallUnary
github.com/bufbuild/connect-go.(*Request[go.shape.struct { github.com/grafana/pyroscope/api/gen/proto/go/push/v1.state google.golang.org/protobuf/internal/impl.MessageState; github.com/grafana/pyroscope/api/gen/proto/go/push/v1.sizeCache int32; github.com/grafana/pyroscope/api/gen/proto/go/push/v1.unknownFields []uint8; Series []*github.com/grafana/pyroscope/api/gen/proto/go/push/v1.RawProfileSeries \"protobuf:\\\"bytes,1,rep,name=series,proto3\\\" json:\\\"series,omitempty\\\"\" }]).setRequestMethod
github.com/bufbuild/connect-go.(*Response[go.shape.struct { github.com/grafana/pyroscope/api/gen/proto/go/push/v1.state google.golang.org/protobuf/internal/impl.MessageState; github.com/grafana/pyroscope/api/gen/proto/go/push/v1.sizeCache int32; github.com/grafana/pyroscope/api/gen/proto/go/push/v1.unknownFields []uint8 }]).Header
github.com/bufbuild/connect-go.NewClient[go.shape.struct { github.com/grafana/pyroscope/api/gen/proto/go/types/v1.state google.golang.org/protobuf/internal/impl.MessageState; github.com/grafana/pyroscope/api/gen/proto/go/types/v1.sizeCache int32; github.com/grafana/pyroscope/api/gen/proto/go/types/v1.unknownFields []uint8; Matchers []string \"protobuf:\\\"bytes,1,rep,name=matchers,proto3\\\" json:\\\"matchers,omitempty\\\"\" },go.shape.struct { github.com/grafana/pyroscope/api/gen/proto/go/types/v1.state google.golang.org/protobuf/internal/impl.MessageState; github.com/grafana/pyroscope/api/gen/proto/go/types/v1.sizeCache int32; github.com/grafana/pyroscope/api/gen/proto/go/types/v1.unknownFields []uint8; Names []string \"protobuf:\\\"bytes,1,rep,name=names,proto3\\\" json:\\\"names,omitempty\\\"\" }].func2
github.com/bufbuild/connect-go.NewRequest[go.shape.struct { github.com/grafana/pyroscope/api/gen/proto/go/push/v1.state google.golang.org/protobuf/internal/impl.MessageState; github.com/grafana/pyroscope/api/gen/proto/go/push/v1.sizeCache int32; github.com/grafana/pyroscope/api/gen/proto/go/push/v1.unknownFields []uint8; Series []*github.com/grafana/pyroscope/api/gen/proto/go/push/v1.RawProfileSeries \"protobuf:\\\"bytes,1,rep,name=series,proto3\\\" json:\\\"series,omitempty\\\"\" }]
github.com/bufbuild/connect-go.NewResponse[go.shape.struct { github.com/grafana/pyroscope/api/gen/proto/go/push/v1.state google.golang.org/protobuf/internal/impl.MessageState; github.com/grafana/pyroscope/api/gen/proto/go/push/v1.sizeCache int32; github.com/grafana/pyroscope/api/gen/proto/go/push/v1.unknownFields []uint8 }]
github.com/bufbuild/connect-go.NewUnaryHandler[go.shape.struct { github.com/grafana/pyroscope/api/gen/proto/go/types/v1.state google.golang.org/protobuf/internal/impl.MessageState; github.com/grafana/pyroscope/api/gen/proto/go/types/v1.sizeCache int32; github.com/grafana/pyroscope/api/gen/proto/go/types/v1.unknownFields []uint8; Name string \"protobuf:\\\"bytes,1,opt,name=name,proto3\\\" json:\\\"name,omitempty\\\"\"; Matchers []string \"protobuf:\\\"bytes,2,rep,name=matchers,proto3\\\" json:\\\"matchers,omitempty\\\"\" },go.shape.struct { github.com/grafana/pyroscope/api/gen/proto/go/types/v1.state google.golang.org/protobuf/internal/impl.MessageState; github.com/grafana/pyroscope/api/gen/proto/go/types/v1.sizeCache int32; github.com/grafana/pyroscope/api/gen/proto/go/types/v1.unknownFields []uint8; Names []string \"protobuf:\\\"bytes,1,rep,name=names,proto3\\\" json:\\\"names,omitempty\\\"\" }].func2
github.com/grafana/pyroscope/pkg/util/loser.(*Tree[go.shape.struct { github.com/grafana/pyroscope/pkg/phlaredb.Profile; Index int },go.shape.interface { At() go.shape.struct { github.com/grafana/pyroscope/pkg/phlaredb.Profile; Index int }; Close() error; Err() error; Next() bool }]).Next
runtime/internal/atomic.(*Pointer[go.shape.struct { runtime.lfnode; runtime.popped runtime/internal/atomic.Uint32; runtime.spans [512]runtime.atomicMSpanPointer }]).Load
sync/atomic.(*Pointer[go.shape.struct { net/http.conn *net/http.conn; net/http.req *net/http.Request; net/http.reqBody io.ReadCloser; net/http.cancelCtx context.CancelFunc; net/http.wroteHeader bool; net/http.wroteContinue bool; net/http.wants10KeepAlive bool; net/http.wantsClose bool; net/http.canWriteContinue sync/atomic.Bool; net/http.writeContinueMu sync.Mutex; net/http.w *bufio.Writer; net/http.cw net/http.chunkWriter; net/http.handlerHeader net/http.Header; net/http.calledHeader bool; net/http.written int64; net/http.contentLength int64; net/http.status int; net/http.closeAfterReply bool; net/http.fullDuplex bool; net/http.requestBodyLimitHit bool; net/http.trailers []string; net/http.handlerDone sync/atomic.Bool; net/http.dateBuf [29]uint8; net/http.clenBuf [10]uint8; net/http.statusBuf [3]uint8; net/http.closeNotifyCh chan bool; net/http.didCloseNotify sync/atomic.Bool }]).Store
github.com/grafana/pyroscope/pkg/phlaredb/symdb.(*deduplicatingSlice[go.shape.*github.com/grafana/pyroscope/pkg/phlaredb/schemas/v1.InMemoryLocation,go.shape.struct { MappingId uint32; Address uint64; LinesHash uint64 },go.shape.*uint8]).ingest
65 changes: 65 additions & 0 deletions pkg/model/tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,71 @@ func (t *Tree) Merge(src *Tree) {
t.root = dstRoot.children
}

func (t *Tree) FormatNodeNames(fn func(string) string) {
nodes := make([]*node, 0, defaultDFSSize)
nodes = append(nodes, &node{children: t.root})
var n *node
var fix bool
for len(nodes) > 0 {
n, nodes = nodes[len(nodes)-1], nodes[:len(nodes)-1]
m := n.name
n.name = fn(m)
if m != n.name {
fix = true
}
nodes = append(nodes, n.children...)
}
if !fix {
return
}
t.Fix()
}

// Fix re-establishes order of nodes and merges duplicates.
func (t *Tree) Fix() {
if len(t.root) == 0 {
return
}
r := &node{children: t.root}
for _, n := range r.children {
n.parent = r
}
nodes := make([][]*node, 0, defaultDFSSize)
nodes = append(nodes, r.children)
var n []*node
for len(nodes) > 0 {
n, nodes = nodes[len(nodes)-1], nodes[:len(nodes)-1]
if len(n) == 0 {
continue
}
sort.Slice(n, func(i, j int) bool {
return n[i].name < n[j].name
})
p := n[0]
j := 1
for _, c := range n[1:] {
if p.name == c.name {
for _, x := range c.children {
x.parent = p
}
p.children = append(p.children, c.children...)
p.total += c.total
p.self += c.self
continue
}
p = c
n[j] = c
j++
}
n = n[:j]
for _, c := range n {
c.parent.children = n
nodes = append(nodes, c.children)
}
}
t.root = r.children
}

func (n *node) String() string {
return fmt.Sprintf("{%s: self %d total %d}", n.name, n.self, n.total)
}
Expand Down
24 changes: 24 additions & 0 deletions pkg/model/tree_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,30 @@ func Test_Tree_MarshalUnmarshal(t *testing.T) {
})
}

func Test_FormatNames(t *testing.T) {
x := newTree([]stacktraces{
{locations: []string{"c0", "b0", "a0"}, value: 3},
{locations: []string{"c1", "b0", "a0"}, value: 3},
{locations: []string{"d0", "b1", "a0"}, value: 2},
{locations: []string{"e1", "c1", "a1"}, value: 4},
{locations: []string{"e2", "c1", "a2"}, value: 4},
})
x.FormatNodeNames(func(n string) string {
if len(n) > 0 {
n = n[:1]
}
return n
})
expected := newTree([]stacktraces{
{locations: []string{"c", "b", "a"}, value: 3},
{locations: []string{"c", "b", "a"}, value: 3},
{locations: []string{"d", "b", "a"}, value: 2},
{locations: []string{"e", "c", "a"}, value: 4},
{locations: []string{"e", "c", "a"}, value: 4},
})
require.Equal(t, expected.String(), x.String())
}

func emptyTree() *Tree {
return &Tree{}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/validation/limits.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func (l *Limits) RegisterFlags(f *flag.FlagSet) {
f.IntVar(&l.MaxProfileStacktraceSamples, "validation.max-profile-stacktrace-samples", 16000, "Maximum number of samples in a profile. 0 to disable.")
f.IntVar(&l.MaxProfileStacktraceSampleLabels, "validation.max-profile-stacktrace-sample-labels", 100, "Maximum number of labels in a profile sample. 0 to disable.")
f.IntVar(&l.MaxProfileStacktraceDepth, "validation.max-profile-stacktrace-depth", 1000, "Maximum depth of a profile stacktrace. Profiles are not rejected instead stacktraces are truncated. 0 to disable.")
f.IntVar(&l.MaxProfileSymbolValueLength, "validation.max-profile-symbol-value-length", 1024, "Maximum length of a profile symbol value (labels, function names and filenames, etc...). Profiles are not rejected instead symbol values are truncated. 0 to disable.")
f.IntVar(&l.MaxProfileSymbolValueLength, "validation.max-profile-symbol-value-length", 65535, "Maximum length of a profile symbol value (labels, function names and filenames, etc...). Profiles are not rejected instead symbol values are truncated. 0 to disable.")
Copy link
Collaborator Author

@kolesnikovae kolesnikovae Aug 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The limit is increased as length of 1024 is too short for Go:

image

In practice we already have profile size limit therefore this one might not be required. Because of the string truncation it's easy to make profiles unreadable (e.g such profiles are useless for PGO)

}

// UnmarshalYAML implements the yaml.Unmarshaler interface.
Expand Down
8 changes: 5 additions & 3 deletions pkg/validation/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,11 @@ func ValidateProfile(limits ProfileValidationLimits, userID string, prof *google
return NewErrorf(SampleLabelsLimit, ProfileTooManySampleLabelsErrorMsg, phlaremodel.LabelPairsString(ls), len(s.Label), labelsLimit)
}
}
for i := range prof.StringTable {
if symbolLengthLimit != 0 && len(prof.StringTable[i]) > symbolLengthLimit {
prof.StringTable[i] = prof.StringTable[i][len(prof.StringTable[i])-symbolLengthLimit:]
if symbolLengthLimit > 0 {
for i := range prof.StringTable {
if len(prof.StringTable[i]) > symbolLengthLimit {
prof.StringTable[i] = prof.StringTable[i][len(prof.StringTable[i])-symbolLengthLimit:]
}
}
}
return nil
Expand Down