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

Traces: Span filtering #65725

Merged
merged 34 commits into from
Apr 17, 2023
Merged

Traces: Span filtering #65725

merged 34 commits into from
Apr 17, 2023

Conversation

joey-grafana
Copy link
Contributor

@joey-grafana joey-grafana commented Mar 31, 2023 •
edited
Loading

What is this feature?

Adds span filtering for the trace view.

Why do we need this feature?

This feature allows users to filter down their trace view. This is especially helpful when the user has a large amount of traces from which they would like to find traces with specific span/operation names, duration ranges, tags etc.

Who is this feature for?

Tracing users.

Special notes for your reviewer:

You will need to set the newTraceView feature toggle to use span filtering.

Span.filters.mov

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.

@joey-grafana joey-grafana self-assigned this Mar 31, 2023
@github-advanced-security
Copy link

You have successfully added a new CodeQL configuration .github/workflows/pr-codeql-analysis-javascript.yml:analyze. As part of the setup process, we have scanned this repository and found 27 existing alerts. Please check the repository Security tab to see all alerts.

@github-actions
Copy link
Contributor

Backend code coverage report for PR #65725
No changes

@github-actions
Copy link
Contributor

github-actions bot commented Mar 31, 2023 •
edited
Loading

Frontend code coverage report for PR #65725

Plugin Main PR Difference
explore 86.34% 86.26% -.08%

@joey-grafana joey-grafana requested review from knylander-grafana and removed request for Eve832 and GrafanaWriter April 4, 2023 17:56
adrapereira
adrapereira requested changes Apr 11, 2023
View reviewed changes
Copy link
Contributor

@adrapereira adrapereira left a comment

Choose a reason for hiding this comment

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

Nice work getting this implemented Joey! Left a few comments and some questions, let me know if you want to discuss anything!

It feels like the filters take too much space, since they're all in a separate row. On one hand this matches what we have in the left-hand side for the query builder, but on the other hand it takes too much vertical space on the trace view, which is already hurting for space. Did you try with the filters arranged in a more horizontal way? Curious if the UX would be good.

public/app/features/explore/TraceView/useSearch.ts Show resolved Hide resolved
public/app/features/explore/TraceView/components/TracePageHeader/NewTracePageHeader.tsx Show resolved Hide resolved
public/app/features/explore/TraceView/components/TracePageHeader/NewTracePageSearchBar.tsx Show resolved Hide resolved
public/app/features/explore/TraceView/components/TracePageHeader/NewTracePageSearchBar.tsx Outdated Show resolved Hide resolved
public/app/features/explore/TraceView/components/TracePageHeader/NewTracePageSearchBar.tsx Outdated Show resolved Hide resolved
public/app/features/explore/TraceView/components/TracePageHeader/SpanFilters/SpanFilters.tsx Show resolved Hide resolved
public/app/features/explore/TraceView/components/TracePageHeader/SpanFilters/SpanFilters.tsx Show resolved Hide resolved
public/app/features/explore/TraceView/components/TracePageHeader/SpanFilters/SpanFilters.tsx Show resolved Hide resolved
public/app/features/explore/TraceView/components/TracePageHeader/SpanFilters/SpanFilters.tsx Outdated Show resolved Hide resolved
public/app/features/explore/TraceView/components/utils/filter-spans.tsx Outdated Show resolved Hide resolved
@joey-grafana
Copy link
Contributor Author

Thanks for the review!

With regards to rendering the inputs inline rather than under each other, I was torn here between saving space and making it easy for the user to scan the UI and modify the inputs they wanted. In the end, I felt that having the inputs places horizontally was too confusing as they were at unexpected horizontal positions and actually shifted as values were entered in the inputs. While it saves space it's difficult for the user to scan across and find the inputs they want. From a UX perspective this is quite messy and positions are variable. Vertically, they know exactly where the inputs will be.

Screenshot 2023-04-12 at 09 58 55
Screenshot 2023-04-12 at 09 59 26

@adrapereira
Copy link
Contributor

We can use CSS improve this and make it responsive to the size of the trace view. If width > x show the fields in a grid with 3 columns, else use 2 or 1 columns. We also need to make sure that they don't change positions while the values change, as you pointed out, the grid should take care of that.

knylander-grafana
knylander-grafana reviewed Apr 13, 2023
View reviewed changes
Copy link
Contributor

@knylander-grafana knylander-grafana left a comment

Choose a reason for hiding this comment

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

Thank you for updating the doc. Sections look good. Very consistent ;) I made some wording and punctuation suggestions.

adrapereira
adrapereira approved these changes Apr 14, 2023
View reviewed changes
Copy link
Contributor

@adrapereira adrapereira left a comment

Choose a reason for hiding this comment

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

Thank you for the changes! Approved 👍

@joey-grafana joey-grafana merged commit 9391700 into main Apr 17, 2023
@joey-grafana joey-grafana deleted the joey/trace-view-span-filtering branch April 17, 2023 07:30
adela-almasan pushed a commit that referenced this pull request Apr 17, 2023
@joey-grafana @adela-almasan
Traces: Span filtering (#65725)
247b5a9 
* Filters

* Service/span/duration filters

* Renames for focused span and matches

* Tag filters and new component

* Tag filtering

* Multiple tags and enable next/prev appropriately

* Enum, renames, fixes

* Clean up unecessary props

* setFocusedSearchMatch

* Faster options

* Perf enhancements and cleanup

* General improvements to tags etc

* Updates to filtering

* Add datasourceType in next/prev

* Integrate TracePageSearchBar with NewTracePageSearchBar

* Design tweaks

* Update sticky elem and header design

* Fix tests

* Self review

* Enhancements

* More enhancements

* Update tests

* tests

* More tests

* Add span filters to docs

* Update image link

* Update docs

* Update buttonEnabled and text

* PR review

* Update sticky header

* Doc updates

* Set values for service/span name

* Buffer and dash update
@joey-grafana joey-grafana mentioned this pull request Apr 24, 2023
Closed
@zerok zerok modified the milestones: 10.0.0, 10.0.0-preview May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@knylander-grafana knylander-grafana knylander-grafana left review comments

@adrapereira adrapereira adrapereira approved these changes

Assignees

@joey-grafana joey-grafana

Labels
add to changelog area/explore area/frontend area/tracing no-backport Skip backport of PR onboarding type/docs
Projects
None yet
Milestone
10.0.0-preview
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@joey-grafana @adrapereira @knylander-grafana @zerok @grafanabot