feat: Add default_spans_per_span_set config and treat spss=0 as unlimited - #5858
Conversation
anglerfishlyy
left a comment
anglerfishlyy
left a comment
There was a problem hiding this comment.
Hi @iamrajiv , overall implementation looks solid, good addition with default_spans_per_span_set , and the docs and tests are covered. a few CI checks are failing (lint , unit , e2e ), likely just needs a make fmt and ensuring ParseSearchRequestWithDefault() is consistently used. once those are fixed , it should be good to go.
javiermolinar
left a comment
javiermolinar
left a comment
There was a problem hiding this comment.
LGTM on minor change. It is required to generate the manifest again
|
Thanks for the update @iamrajiv,changes look good after formatting. Agree with @javiermolinar’s suggestion on handling negative values for safety. You can also run |
knylander-grafana
left a comment
knylander-grafana
left a comment
There was a problem hiding this comment.
Thank you for updating the docs!
|
@javiermolinar @knylander-grafana can we run the ci again? |
javiermolinar
left a comment
javiermolinar
left a comment
There was a problem hiding this comment.
LGTM thank you for your contribution
…ited (grafana#5858) * add config * add search config too * add test * add changelog * add docs * fix typos * fmt * add manifest * revert
4 participants
What this PR does:
Add default_spans_per_span_set config and treat spss=0 as unlimited
Which issue(s) this PR fixes:
Fixes #5435
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]