Add CLI configurable MaxNumSpans while retrieving spans from ES.

[annanay25]

Signed-off-by: Annanay [email protected]

Which problem is this PR solving?

• Follow up PR for [query] Truncate search results to prevent OOM on jaeger-query. #1202
• Attempts to resolve Jaeger Query can OOM when retrieving large traces #1051

Short description of the changes

• Added a CLI configurable option to limit the number of spans fetched in a single query to the ES backend.

[black-adder]

My only question is are we ok with changing behavior that people are used to? If people upgrade jaeger, they'll automatically be hit with the 10000 limit and not know about it. Perhaps if no limit is set, we don't apply a default limit?

[codecov]

Codecov Report

Merging #1283 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           master   #1283   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files         161     161           
  Lines        7198    7206    +8     
======================================
+ Hits         7198    7206    +8

Impacted Files	Coverage Δ
plugin/storage/es/spanstore/reader.go	100% <100%> (ø)	⬆️
plugin/storage/es/options.go	100% <100%> (ø)	⬆️
plugin/storage/es/factory.go	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 48b034c...9e4bb5f. Read the comment docs.

[annanay25]

@black-adder; this makes sense.

However, we could mention the reasons for adopting such a limit in the docs (that it could overwhelm the query-service) and mention that it could be overwritten using the flag in the release notes.

[yurishkuro]

please add to the changelog.

is the behavior actually different from before? I thought we already had another 10k limit in the code.

[yurishkuro]

This is being reverted in #4336