Add maximum spans per span set #4383
Conversation
carles-grafana
force-pushed
the
limit-spans
branch
from
56457fa to
da73759
Compare
carles-grafana
requested review from
knylander-grafana,
mdisibio,
mapno,
yvrhdn,
zalegrala,
electron0zero,
ie-pham,
stoewer and
javiermolinar
as code owners
|
|
||
|
|
||
| # The maximum allowed value of spans per span set. 0 disables this limit. | ||
| [max_spans_per_span_set]: <int> | default = 100] |
There was a problem hiding this comment.
Should we set it to 0 by default? then it won't be a breaking change
There was a problem hiding this comment.
I'm not opposed to 0. If you all prefer it, we can do that.
However, this feels like a sensible default that will help all OSS users so i'd prefer the breaking change that just makes Tempo operate better.
There was a problem hiding this comment.
That's okay to me. It simplify setting up Tempo for sure
There was a problem hiding this comment.
I think this needs more context so people know what a limit of 100 spansets mean. Are we talking about results of a traceql query?
There was a problem hiding this comment.
this is a limit of 100 spans per spanset. it would be this field in Grafana:
naturally this can be adjusted to whatever works for your org (or turned off with 0)
| @@ -731,6 +732,12 @@ func TestSearchSharderRoundTripBadRequest(t *testing.T) { | |||
| resp, err = testRT.RoundTrip(pipeline.NewHTTPRequest(req)) | |||
| testBadRequestFromResponses(t, resp, err, "range specified by start and end exceeds 5m0s. received start=1000 end=1500") | |||
|
|
|||
| // spans per span set greater than maximum | |||
| req = httptest.NewRequest("GET", "/?spss=200", nil) | |||
| req = req.WithContext(user.InjectOrgID(req.Context(), "blerg")) | |||
There was a problem hiding this comment.
"blerg". well, i guess you're officially on the team now :)
This as a breaking change.
carles-grafana
force-pushed
the
limit-spans
branch
from
3f0154b to
c9084a0
Compare
What this PR does:
Add maximum spans per span set.
Which issue(s) this PR fixes:
Fixes #4275
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]