feat: Add default_spans_per_span_set config and treat spss=0 as unlimited#5858
feat: Add default_spans_per_span_set config and treat spss=0 as unlimited#5858javiermolinar merged 11 commits intografana:mainfrom
Conversation
iamrajiv
requested review from
carles-grafana,
electron0zero,
ie-pham,
javiermolinar,
joe-elliott,
mapno,
mattdurham,
mdisibio,
ruslan-mikhailov,
stoewer,
yvrhdn and
zalegrala
as code owners
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
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]