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.

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

Add filter to homepage #326

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Add filter to homepage #326

wants to merge 6 commits into from

Conversation

joey-grafana
Copy link
Collaborator

@joey-grafana joey-grafana commented Feb 6, 2025

Internal user interviews showed us that users would like the ability to select which service/cluster/etc that are most interested in or simply to see errors/slow traces for an attribute other than resource.service.name. Having spoken with @nadinevehling & @alexbikfalvi, we decided to allow users to add an optional filter for this iteration and based on feedback we will decide if we want to allow them to add more than one filter/create views (something like what Explore Logs does), but for this iteration it was requested to walk before we run :D

Fixes grafana/grafana#99862

Screenshot 2025-02-17 at 14 20 42

@joey-grafana joey-grafana marked this pull request as ready for review February 7, 2025 09:13
@joey-grafana joey-grafana requested a review from a team February 7, 2025 09:13
@nadinevehling
Copy link

@joey-grafana Small feedback, I'd enlarge the spacing between the data source input and the text "filter" sightly to distinguish both filters. Right now, it's hard to tell which belongs together.

@alexbikfalvi
Copy link
Contributor

@joey-grafana @nadinevehling Would it be easy that instead of a single filter option we use the same filter that we have in the app?

Screenshot 2025-02-10 at 11 49 21 AM

Rationale: Keep the same experience that we have in the app, the app filter is more flexible, and we would avoid having to create a new set of interactions.


Feedback on the current filter:

  • Once I choose a filter attribute, there is no way to clear it.
  • When selecting a filter attribute, the number of errors seems incorrect (it only shows "1")

Screenshot 2025-02-10 at 12 01 29 PM

Minor comment: When I selected an attribute in the filter, the "Errors" pane reconfigured to show the errors breakdown by the selected attribute even if I haven't entered an attribute value. However, the "Slow traces" pane didn't change until I selected also a value. This was slightly confusing not knowing if it was a bug or not.

@alexbikfalvi
Copy link
Contributor

alexbikfalvi commented Feb 18, 2025

Fantastic work @joey-grafana - leaving a few individual comments below.

Error for certain filters

There is an error for certain filters, such as span:name = add-checkout for instance.
Screenshot 2025-02-18 at 11 34 45 AM

The query is the following:

http://localhost:3000/api/datasources/proxy/uid/tempo/api/metrics/query_range?query=%7BnestedSetParent%20%3C%200%20%26%26%20status%20%3D%20error%20%26%26%20span%3Aname%3Dadd-checkout%7D%20%7C%20count_over_time()%20by%20(resource.service.name)&start=1739873095&end=1739874895&step=1800s

answered with 400 Bad Request - compiling query: parse error at line 1, col 53: syntax error: unexpected IDENTIFIER

This could be because of missing quotes around add-checkout, the following query works:

http://localhost:3000/api/datasources/proxy/uid/tempo/api/metrics/query_range?query=%7BnestedSetParent%20%3C%200%20%26%26%20status%20%3D%20error%20%26%26%20span%3Aname%3D%22add-checkout%22%7D%20%7C%20count_over_time()%20by%20(resource.service.name)&start=1739873095&end=1739874895&step=1800s

I think we have a similar issue in the app filter, but there the quotes are added for non-string values. Happy to park this and try to tackle separately.

Not possible to type regex

When choosing a regex match, I can only select from the drop down, but not enter a regex.

Screenshot 2025-02-18 at 11 39 44 AM

If too complex to enable typical values, perhaps we could leave regex out for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add quick-use filters to Explore Traces Homepage
3 participants