Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tempo: Added spss config - spans per span set #74832

Merged
merged 4 commits into from Sep 14, 2023
Merged

Tempo: Added spss config - spans per span set #74832

merged 4 commits into from Sep 14, 2023

Conversation

adrapereira
Copy link
Contributor

What is this feature?

Adds a new option to the Search and TraceQL tabs to control the limit of spans per span set.

Which issue(s) does this PR fix?:

Fixes #68477

Special notes for your reviewer:

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@adrapereira adrapereira added this to the 10.2.x milestone Sep 13, 2023
@adrapereira adrapereira self-assigned this Sep 13, 2023
@adrapereira adrapereira requested a review from a team as a code owner September 13, 2023 12:50
@adrapereira adrapereira changed the title Tempo - Added spss config - spans per span set Tempo: Added spss config - spans per span set Sep 13, 2023
joey-grafana
joey-grafana reviewed Sep 14, 2023
View reviewed changes
Copy link
Contributor

@joey-grafana joey-grafana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • First load (and subsequent loads) of Tempo on the branch causes this error. Just have to set spss once but should have it set by default for API call/avoid error on first load.
    Screenshot 2023-09-14 at 09 37 12

  • Value appears to be ignored for streaming
    Screenshot 2023-09-14 at 09 44 32

@adrapereira
Copy link
Contributor Author

First load (and subsequent loads) of Tempo on the branch causes this error. Just have to set spss once but should have it set by default for API call/avoid error on first load.

Fixed, thanks for spotting this!

Value appears to be ignored for streaming

This is a bug on the Tempo side and it will be fixed soon.

@adrapereira adrapereira merged commit dc44ac7 into main Sep 14, 2023
19 checks passed
@adrapereira adrapereira deleted the andre/add-spss-to-tempo-query-options branch September 14, 2023 09:49
chauchausoup pushed a commit to chauchausoup/grafana that referenced this pull request Sep 15, 2023
@adrapereira @chauchausoup
Tempo: Added spss config - spans per span set (grafana#74832)
277b390 
* Added spss config - spans per span set

* Set default spss when calling Tempo
rwwiv pushed a commit that referenced this pull request Oct 2, 2023
@adrapereira @rwwiv
Tempo: Added spss config - spans per span set (#74832)
9bbf02f 
* Added spss config - spans per span set

* Set default spss when calling Tempo
@zerok zerok modified the milestones: 10.2.x, 10.2.0 Oct 23, 2023
@dnhn dnhn mentioned this pull request Oct 24, 2023
Closed
@BrewTestBot BrewTestBot mentioned this pull request Oct 25, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joey-grafana joey-grafana joey-grafana approved these changes

Assignees

@adrapereira adrapereira

Labels
add to changelog area/backend area/datasource/frontend area/frontend datasource/Tempo no-backport Skip backport of PR
Projects
None yet
Milestone
10.2.0
Development

Successfully merging this pull request may close these issues.

[TraceQL] Support spans per spanset
3 participants
@adrapereira @joey-grafana @zerok