tempo searchsharding: ensure limit is never exceeded

What this PR does: Ensure the search response from Tempo always contain at most limit traces. We were missing a check when appending results to the response object, this way additional traces could sneak into the response object.

Example: you already have 5 results, limit is 6. If a subquery returns 2 more results we would end up with 7 results. By checking the limit every time a trace is appended this won't happen anymore.

Which issue(s) this PR fixes: ~~Fixes #~~

Checklist

[x] Tests updated
[ ] Documentation added
[ ] CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Oct 26 '22 18:10 yvrhdn

This was originally written this way intentionally. If we've already done the work to pull additional traces why throw them away? However, we've gotten so many questions about this that it probably makes sense to go ahead with this change.

Oct 26 '22 19:10 joe-elliott

This was originally written this way intentionally. If we've already done the work to pull additional traces why throw them away? However, we've gotten so many questions about this that it probably makes sense to go ahead with this change.

I see 😅 Yeah, I agree with that reasoning, we already fetched this data from the queriers so why not just return it. I created the PR because we got reports of traceQL queries returning more than the limit.

I guess it depends a bit on how we view limit: is this a value to protect the amount of data the caller receives or is this a value to protect the backend? We mostly see it as the latter, but I guess most APIs use limit in combination with pagination to limit response size. Hence the confusion.

Oct 26 '22 19:10 yvrhdn

This PR has been automatically marked as stale because it has not had any activity in the past 60 days. The next time this stale check runs, the stale label will be removed if there is new activity. This pull request will be closed in 15 days if there is no new activity. Please apply keepalive label to exempt this Pull Request.

Dec 26 '22 00:12 github-actions[bot]

Fixes #1392

Jan 03 '23 15:01 joe-elliott

This PR has been automatically marked as stale because it has not had any activity in the past 60 days. The next time this stale check runs, the stale label will be removed if there is new activity. This pull request will be closed in 15 days if there is no new activity. Please apply keepalive label to exempt this Pull Request.

Mar 05 '23 00:03 github-actions[bot]