Logs: Show active state of "filter for value" buttons in Logs Details #70328

matyax · 2023-06-19T14:07:18Z

This PR adds support to showing the active state of filter for value filters within Logs Context.

This was done through the addition of the Datasource interface DataSourceToggleableQueryFiltersSupport two new methods:

toggleQueryFilter(query: TQuery, filter: ToggleFilterAction): TQuery;
queryHasFilter(query: TQuery, filter: QueryFilterOptions): boolean;

This change is part of a bigger set of changes, aiming to make looking for logs (search and filtering) easier for users.

What issue this PR solves

Part of #61765
Part of #70091

Special notes for your reviewer:

You need to enable the following feature flag: toggleLabelsInLogsUI

Loki demo:

Elastic demo:

github-actions · 2023-06-19T15:26:02Z

Backend code coverage report for PR #70328
No changes

github-actions · 2023-06-19T15:26:04Z

Frontend code coverage report for PR #70328

Plugin	Main	PR	Difference
explore	86.5%	86.34%	-.16%
elasticsearch	83.07%	83.04%	-.03%
loki	85.29%	85.32%	.03%

gelicia · 2023-06-21T22:54:28Z

For Explore, I'm a little concerned because we have an inspector button with a query tab that this this inspectQuery has nothing to do with. I think a comment in Explore.tsx would go a long way. Further, I think that all the functions in Explore.tsx that are logs only at the moment could have a comment about their usage so its easy to see they're specific for logs. We might want to move them to a Logs component someday. At a quick glance, I think those functions are inspectQuery, isFIlterLabelActive, onClickFilterLabel, and onClickFilterOutLabel.

Additionally, I'm wondering what you think about changing the inspectQuery function so instead of calling it once per field when expanding a log line (I was seeing it called 10 times), that InspectQueryOptions's attributes take in an array of key/values and it would return an array of keys with their respective booleans saying if that field has a filter or not.

matyax · 2023-06-22T08:56:12Z

Thanks for the feedback @gelicia !

For Explore, I'm a little concerned because we have an inspector button with a query tab that this this inspectQuery has nothing to do with. I think a comment in Explore.tsx would go a long way.

Good idea. I will also consider other names to prevent confusing.

Further, I think that all the functions in Explore.tsx that are logs only at the moment could have a comment about their usage so its easy to see they're specific for logs. We might want to move them to a Logs component someday.

Great point, will add it.

Additionally, I'm wondering what you think about changing the inspectQuery function so instead of calling it once per field when expanding a log line (I was seeing it called 10 times), that InspectQueryOptions's attributes take in an array of key/values and it would return an array of keys with their respective booleans saying if that field has a filter or not.

Great idea. I will explore it and get back to you.

packages/grafana-data/src/types/datasource.ts

public/app/features/explore/Explore.tsx

public/app/features/explore/Logs/LogsContainer.tsx

public/app/features/explore/Explore.tsx

matyax · 2023-07-20T16:10:36Z

I ran out of time to update the tests, but I will do it tomorrow, as I would really like to have this before 10.1.

ivanahuckova

I like this implementation a lot! 🤩 Left couple of comments mostly related to naming & comments, but as soon as those are resolved and tests are updated LGTM!

packages/grafana-data/src/types/logs.ts

pkg/services/featuremgmt/registry.go

public/app/features/explore/Explore.tsx

ivanahuckova

LGTM 🚀

matyax · 2023-07-21T12:03:54Z

All the breaking changes have been removed from these changes, so I'll go ahead and merge this. Thank you so much everyone for your input and feedback in this PR.

…#70328) * Datasource test: fix describe nesting * Parsing: export handleQuotes function * Modify query: add functions to detect the presence of a label and remove it * Loki: add support to toggle filters if already present * Datasource test: fix describe nesting * Loki: add support to toggle filter out if present * Remove label: handle escaped values * Datasource: add test case for escaped label values * Loki: remove = filter when applying != * Remove selector: add support for Selector node being far from Matcher * Modify query: add unit tests * Elasticsearch: create modifyQuery for elastic * Elastic modify query: implement functions * Elasticsearch: implement modifyQuery functions in datasource * Elasticsearch: update datasource test * Loki modify query: check for streamSelectorPositions length * Elasticsearch query has filter: escape filter value in regex * Remove unused type * Modify query: add functions to detect the presence of a label and remove it * Remove label: handle escaped values * Logs: create props to check for label filters in the query * Log Details Row: use label state props to show visual feedback * Make isCallbacks async * Explore: add placeholder for checking for filter in query * Datasource: define new API method * Inspect query: add base implementation * Remove isFilterOutLabelActive as it will not be needed * Check for "isActive" on every render Otherwise the active state will be out of sync * Elasticsearch: implement inspectQuery in the datasource * Logs: update test * Log details: update test * Datasources: update tests * Inspect query: rename to analize query to prevent confusion * Datasource types: mark method as alpha * Explore: add comment to log-specific functions * Remove duplicated code from bad rebase * Remove label filter: check node type * getMatchersWithFilter: rename argument * Fix bad rebase * Create DataSourceWithQueryManipulationSupport interface * Implement type guard for DataSourceWithQueryManipulationSupport * DataSourceWithQueryManipulationSupport: move to logs module * hasQueryManipulationSupport: change implementation `modifyQuery` comes from the prototype. * DataSourceWithQueryManipulationSupport: expand code comments * AnalyzeQueryOptions: move to logs module * DataSourceWithQueryManipulationSupport: add support for more return types * Fix merge error * Update packages/grafana-data/src/types/logs.ts Co-authored-by: Sven Grossmann <sven.grossmann@grafana.com> * DatasourceAPI: deprecate modifyQuery * Explore: refactor isFilterLabelActive * DataSourceWithQueryModificationSupport: rename interface * Split interfaces into Analyze and Modify * Query analysis: better name for interface * Fix guard * Create feature flag for active state * Use new feature flag in Explore * DataSourceToggleableQueryFiltersSupport: create a specific interface for this feature * Rename feature flag * De-deprecate modifyQuery * DataSourceToggleableQueryFiltersSupport: Rethink types and methods * Explore: adjust modifyQuery and isFilterLabelActive to new methods * Loki: implement new interface and revert modifyQuery * DataSourceToggleableQueryFiltersSupport: better name for arguments * Elasticsearch: implement new interface and revert modifyQuery * Loki: better name for arguments * Explore: document current limitation on isFilterLabelActive * Explore: place toggleable filters under feature flag * Loki: add tests for the new methods * Loki: add legacy modifyQuery tests * Elasticsearch: add tests for the new methods * Elasticsearch: add legacy modifyQuery tests * Toggle filter action: improve type values * Logs types: update interface description * DataSourceWithToggleableQueryFiltersSupport: update interface name * Update feature flag description * Explore: add todo comment for isFilterLabelActive --------- Co-authored-by: Sven Grossmann <sven.grossmann@grafana.com>

grafanabot added datasource/Loki datasource/Elasticsearch area/explore area/frontend labels Jun 19, 2023

matyax changed the base branch from main to matyax/log-details-toggle-off June 19, 2023 14:31

matyax changed the title ~~WIP: Logs: Show active state of "filter for value" buttons~~ Logs: Show active state of "filter for value" buttons Jun 19, 2023

matyax marked this pull request as ready for review June 19, 2023 15:22

matyax requested review from a team as code owners June 19, 2023 15:22

matyax requested review from tskarhed, JoaoSilvaGrafana and mckn and removed request for a team June 19, 2023 15:22

matyax added add to changelog no-backport Skip backport of PR labels Jun 19, 2023

matyax added this to the 10.1.x milestone Jun 19, 2023

matyax changed the title ~~Logs: Show active state of "filter for value" buttons~~ Logs: Show active state of "filter for value" buttons in Logs Details Jun 19, 2023

matyax requested review from svennergr and ivanahuckova June 20, 2023 15:39

ifrost reviewed Jun 22, 2023

View reviewed changes

matyax mentioned this pull request Jun 23, 2023

Logs: Integrate table view with filters with active state #70584

Open

matyax added 2 commits June 26, 2023 11:12

Datasource test: fix describe nesting

c352b00

Parsing: export handleQuotes function

470ac3c

Explore: document current limitation on isFilterLabelActive

d259ca7

matyax requested a review from ifrost July 20, 2023 16:04

Explore: place toggleable filters under feature flag

400cc83

matyax requested a review from ivanahuckova July 20, 2023 16:10

matyax added 4 commits July 21, 2023 10:47

Loki: add tests for the new methods

e1bf0eb

Loki: add legacy modifyQuery tests

9f9d8e6

Elasticsearch: add tests for the new methods

aa56ee3

Elasticsearch: add legacy modifyQuery tests

f89fda8

ivanahuckova reviewed Jul 21, 2023

View reviewed changes

matyax added 5 commits July 21, 2023 11:26

Toggle filter action: improve type values

23e759f

Logs types: update interface description

0c1e80d

DataSourceWithToggleableQueryFiltersSupport: update interface name

9289fb5

Update feature flag description

6a44bb1

Explore: add todo comment for isFilterLabelActive

e0d0b14

matyax requested a review from ivanahuckova July 21, 2023 09:45

ivanahuckova approved these changes Jul 21, 2023

View reviewed changes

Merge branch 'main' into matyax/log-details-filters-toggle

da86f5c

matyax enabled auto-merge (squash) July 24, 2023 08:13

github-actions bot added the pr/external This PR is from external contributor label Jul 24, 2023

matyax merged commit 84f94cd into main Jul 24, 2023
16 of 17 checks passed

matyax deleted the matyax/log-details-filters-toggle branch July 24, 2023 08:22

This was referenced Jul 27, 2023

Feature toggle removal: toggleLabelsInLogsUI #72437

Closed

Logs: Fix ui getting stuck when removing fields #72597

Merged

grafana-delivery-bot bot mentioned this pull request Jul 31, 2023

[v10.1.x] Logs: Fix ui getting stuck when removing fields #72603

Merged

ricky-undeadcoders modified the milestones: 10.1.x, 10.1.0 Aug 1, 2023

chenrui333 mentioned this pull request Aug 24, 2023

grafana 10.1.0 Homebrew/homebrew-core#140321

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Logs: Show active state of "filter for value" buttons in Logs Details #70328

Logs: Show active state of "filter for value" buttons in Logs Details #70328

matyax commented Jun 19, 2023 •

edited

github-actions bot commented Jun 19, 2023

github-actions bot commented Jun 19, 2023 •

edited

gelicia commented Jun 21, 2023

matyax commented Jun 22, 2023

matyax commented Jul 20, 2023

ivanahuckova left a comment

ivanahuckova left a comment

matyax commented Jul 21, 2023

Logs: Show active state of "filter for value" buttons in Logs Details #70328

Logs: Show active state of "filter for value" buttons in Logs Details #70328

Conversation

matyax commented Jun 19, 2023 • edited

github-actions bot commented Jun 19, 2023

github-actions bot commented Jun 19, 2023 • edited

gelicia commented Jun 21, 2023

matyax commented Jun 22, 2023

matyax commented Jul 20, 2023

ivanahuckova left a comment

Choose a reason for hiding this comment

ivanahuckova left a comment

Choose a reason for hiding this comment

matyax commented Jul 21, 2023

matyax commented Jun 19, 2023 •

edited

github-actions bot commented Jun 19, 2023 •

edited