Skip to content

Commit 9bc2847

Browse files
authored
feat: Allow to optionally enforce maxNodes through limits (#4436)
1 parent 8471127 commit 9bc2847

File tree

8 files changed

+154
-32
lines changed

8 files changed

+154
-32
lines changed

cmd/pyroscope/help-all.txt.tmpl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,8 @@ Usage of ./pyroscope:
579579
Maximum number of flame graph nodes by default. 0 to disable. (default 8192)
580580
-querier.max-flamegraph-nodes-max int
581581
Maximum number of flame graph nodes allowed. 0 to disable. (default 1048576)
582+
-querier.max-flamegraph-nodes-on-select-merge-profile
583+
Enforce the max nodes limits and defaults on SelectMergeProfile API. Historically this limit was not enforced to enable to gather full pprof profiles without truncation.
582584
-querier.max-query-length duration
583585
The limit to length of queries. 0 to disable. (default 1d)
584586
-querier.max-query-lookback duration

pkg/frontend/frontend_diff_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ func (m *mockLimits) MaxFlameGraphNodesMax(_ string) int {
5151
return 100_000
5252
}
5353

54+
func (m *mockLimits) MaxFlameGraphNodesOnSelectMergeProfile(_ string) bool {
55+
return true
56+
}
57+
5458
func (m *mockLimits) SymbolizerEnabled(s string) bool { return true }
5559

5660
type mockRoundTripper struct {

pkg/frontend/frontend_select_merge_profile.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,21 @@ func (f *Frontend) SelectMergeProfile(
4848
// NOTE: Max nodes limit is not set by default:
4949
// the method is used for pprof export and
5050
// truncation is not applicable for that.
51+
maxNodesEnabled := false
52+
for _, tenantID := range tenantIDs {
53+
if f.limits.MaxFlameGraphNodesOnSelectMergeProfile(tenantID) {
54+
maxNodesEnabled = true
55+
}
56+
}
57+
58+
maxNodes := c.Msg.MaxNodes
59+
if maxNodesEnabled {
60+
maxNodesV, err := validation.ValidateMaxNodes(f.limits, tenantIDs, c.Msg.GetMaxNodes())
61+
if err != nil {
62+
return nil, connect.NewError(connect.CodeInvalidArgument, err)
63+
}
64+
maxNodes = &maxNodesV
65+
}
5166

5267
var m pprof.ProfileMerge
5368
for intervals.Next() {
@@ -58,7 +73,7 @@ func (f *Frontend) SelectMergeProfile(
5873
LabelSelector: c.Msg.LabelSelector,
5974
Start: r.Start.UnixMilli(),
6075
End: r.End.UnixMilli(),
61-
MaxNodes: c.Msg.MaxNodes,
76+
MaxNodes: maxNodes,
6277
StackTraceSelector: c.Msg.StackTraceSelector,
6378
})
6479
resp, err := connectgrpc.RoundTripUnary[

pkg/test/mocks/mockfrontend/mock_limits.go

Lines changed: 92 additions & 17 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/test/mocks/mockobjstore/mock_bucket.go

Lines changed: 24 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/validation/limits.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,9 @@ type Limits struct {
8787
QueryAnalysisSeriesEnabled bool `yaml:"query_analysis_series_enabled" json:"query_analysis_series_enabled"`
8888

8989
// Flame graph enforced limits.
90-
MaxFlameGraphNodesDefault int `yaml:"max_flamegraph_nodes_default" json:"max_flamegraph_nodes_default"`
91-
MaxFlameGraphNodesMax int `yaml:"max_flamegraph_nodes_max" json:"max_flamegraph_nodes_max"`
92-
90+
MaxFlameGraphNodesDefault int `yaml:"max_flamegraph_nodes_default" json:"max_flamegraph_nodes_default"`
91+
MaxFlameGraphNodesMax int `yaml:"max_flamegraph_nodes_max" json:"max_flamegraph_nodes_max"`
92+
MaxFlameGraphNodesOnSelectMergeProfile bool `yaml:"max_flamegraph_nodes_on_select_merge_profile" json:"max_flamegraph_nodes_on_select_merge_profile" category:"advanced" doc:"hidden"`
9393
// Store-gateway.
9494
StoreGatewayTenantShardSize int `yaml:"store_gateway_tenant_shard_size" json:"store_gateway_tenant_shard_size"`
9595

@@ -186,6 +186,7 @@ func (l *Limits) RegisterFlags(f *flag.FlagSet) {
186186

187187
f.IntVar(&l.MaxFlameGraphNodesDefault, "querier.max-flamegraph-nodes-default", 8<<10, "Maximum number of flame graph nodes by default. 0 to disable.")
188188
f.IntVar(&l.MaxFlameGraphNodesMax, "querier.max-flamegraph-nodes-max", 1<<20, "Maximum number of flame graph nodes allowed. 0 to disable.")
189+
f.BoolVar(&l.MaxFlameGraphNodesOnSelectMergeProfile, "querier.max-flamegraph-nodes-on-select-merge-profile", false, "Enforce the max nodes limits and defaults on SelectMergeProfile API. Historically this limit was not enforced to enable to gather full pprof profiles without truncation.")
189190

190191
f.Var(&l.DistributorAggregationWindow, "distributor.aggregation-window", "Duration of the distributor aggregation window. Requires aggregation period to be specified. 0 to disable.")
191192
f.Var(&l.DistributorAggregationPeriod, "distributor.aggregation-period", "Duration of the distributor aggregation period. Requires aggregation window to be specified. 0 to disable.")
@@ -428,6 +429,11 @@ func (o *Overrides) MaxFlameGraphNodesMax(tenantID string) int {
428429
return o.getOverridesForTenant(tenantID).MaxFlameGraphNodesMax
429430
}
430431

432+
// MaxFlameGraphNodesOnSelectMergeProfiles returns if the max flame graph nodes should be enforced for the SelectMergeProfile API.
433+
func (o *Overrides) MaxFlameGraphNodesOnSelectMergeProfile(tenantID string) bool {
434+
return o.getOverridesForTenant(tenantID).MaxFlameGraphNodesOnSelectMergeProfile
435+
}
436+
431437
// StoreGatewayTenantShardSize returns the store-gateway shard size for a given user.
432438
func (o *Overrides) StoreGatewayTenantShardSize(userID string) int {
433439
return o.getOverridesForTenant(userID).StoreGatewayTenantShardSize

pkg/validation/testutil.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,9 @@ type MockLimits struct {
1616
MaxLabelValueLengthValue int
1717
MaxLabelNamesPerSeriesValue int
1818

19-
MaxFlameGraphNodesDefaultValue int
20-
MaxFlameGraphNodesMaxValue int
19+
MaxFlameGraphNodesDefaultValue int
20+
MaxFlameGraphNodesMaxValue int
21+
MaxFlameGraphNodesOnSelectMergeProfileValue bool
2122

2223
DistributorAggregationWindowValue time.Duration
2324
DistributorAggregationPeriodValue time.Duration
@@ -51,6 +52,9 @@ func (m MockLimits) QuerySanitizeOnMerge(tenantID string) bool {
5152
}
5253
func (m MockLimits) MaxFlameGraphNodesDefault(string) int { return m.MaxFlameGraphNodesDefaultValue }
5354
func (m MockLimits) MaxFlameGraphNodesMax(string) int { return m.MaxFlameGraphNodesMaxValue }
55+
func (m MockLimits) MaxFlameGraphNodesOnSelectMergeProfile(string) bool {
56+
return m.MaxFlameGraphNodesOnSelectMergeProfileValue
57+
}
5458

5559
func (m MockLimits) MaxLabelNameLength(userID string) int { return m.MaxLabelNameLengthValue }
5660
func (m MockLimits) MaxLabelValueLength(userID string) int { return m.MaxLabelValueLengthValue }

pkg/validation/validate.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,7 @@ func SanitizeTimeRange(limits RangeRequestLimits, tenant []string, start, end *i
517517
type FlameGraphLimits interface {
518518
MaxFlameGraphNodesDefault(string) int
519519
MaxFlameGraphNodesMax(string) int
520+
MaxFlameGraphNodesOnSelectMergeProfile(string) bool
520521
}
521522

522523
func ValidateMaxNodes(l FlameGraphLimits, tenantIDs []string, n int64) (int64, error) {

0 commit comments

Comments
 (0)