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

Transformations: Extended support for variables in filter by name #75734

Merged
merged 7 commits into from
Oct 3, 2023
Merged

Transformations: Extended support for variables in filter by name #75734

merged 7 commits into from
Oct 3, 2023

Conversation

oscarkilhed
Copy link
Contributor

What is this feature?
This adds support for using variables with multiple values in the filter by name transformation.
image

Peek 2023-09-29 12-19

Fixes #74488

The original feature request specified this behavior on organize fields. But the functionality fits a lot better in filter by name.

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.

@oscarkilhed oscarkilhed added this to the 10.2.x milestone Sep 29, 2023
@oscarkilhed oscarkilhed requested review from grafanabot and a team as code owners September 29, 2023 10:23
@oscarkilhed oscarkilhed requested review from ashharrison90, JoaoSilvaGrafana, leventebalogh, a team, codeincarnate and baldm0mma and removed request for a team September 29, 2023 10:23
@grafana-pr-automation grafana-pr-automation bot added the type/docs Flags the technical writing team for documentation support; auto adds to org-wide docs project label Sep 29, 2023
imatwawana
imatwawana reviewed Sep 29, 2023
View reviewed changes
Copy link
Collaborator

@imatwawana imatwawana left a comment

Choose a reason for hiding this comment

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

Thanks for this addition! I made some small restructuring edits and a number of wording/style edits.

docs/sources/panels-visualizations/query-transform-data/transform-data/index.md Outdated Show resolved Hide resolved
docs/sources/panels-visualizations/query-transform-data/transform-data/index.md Outdated Show resolved Hide resolved
docs/sources/panels-visualizations/query-transform-data/transform-data/index.md Outdated Show resolved Hide resolved
docs/sources/panels-visualizations/query-transform-data/transform-data/index.md Outdated Show resolved Hide resolved
docs/sources/panels-visualizations/query-transform-data/transform-data/index.md Outdated Show resolved Hide resolved
docs/sources/panels-visualizations/query-transform-data/transform-data/index.md Outdated Show resolved Hide resolved
docs/sources/panels-visualizations/query-transform-data/transform-data/index.md Outdated Show resolved Hide resolved
docs/sources/panels-visualizations/query-transform-data/transform-data/index.md Outdated Show resolved Hide resolved
oscarkilhed and others added 2 commits October 2, 2023 09:10
@oscarkilhed @imatwawana
Update docs/sources/panels-visualizations/query-transform-data/transf…
96ec5f3 
…orm-data/index.md

Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>
@oscarkilhed @imatwawana
Apply suggestions from code review
ba1576f 
Co-authored-by: Isabel <76437239+imatwawana@users.noreply.github.com>
@oscarkilhed oscarkilhed requested a review from mdvictor October 2, 2023 07:13
mdvictor
mdvictor reviewed Oct 2, 2023
View reviewed changes
packages/grafana-data/src/transformations/transformers/filterByName.ts
const n = stringOfNames.slice(1).slice(0, -1).split(',');
return { id: FieldMatcherID.byNames, options: { names: n } };
}
return { id: FieldMatcherID.byNames, options: { names: stringOfNames.split(',') } };
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need the split(',') at the end here? Seems that this path will only result in single option values from my understanding.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This path is if you have a variable where the value is a comma separated list of names. Maybe not the most useful scenario as it can only really happen if the user inputs it manually 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand, thanks! Not sure if we'd need some more explanatory docs around this, as it seems to me a bit like a hidden feature, but I'll leave that decision to you.

mdvictor
mdvictor reviewed Oct 2, 2023
View reviewed changes
Copy link
Contributor

@mdvictor mdvictor left a comment

Choose a reason for hiding this comment

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

This works nicely! Left a few comments, but otherwise this LGTM!

@oscarkilhed oscarkilhed requested a review from mdvictor October 2, 2023 18:27
@oscarkilhed oscarkilhed merged commit e0919a3 into main Oct 3, 2023
13 checks passed
@oscarkilhed oscarkilhed deleted the oscark/extend-filter-support-for-filter-by-name branch October 3, 2023 13:06
bergquist added a commit that referenced this pull request Oct 3, 2023
@bergquist
Merge branch 'main' into instrumentations_enable_native_histogram
43553b2 
* main:
  Docs: Fix link to developing plugins (#75816)
  Fix: visualization vs visualisation in feature description (#75895)
  Chore: Bump storybook 7.4.5 (#75652)
  Correlations: Add an editor in Explore (#73315)
  i18n: dashboard settings (#75854)
  Tempo: Highlight errors in TraceQL query (#74697)
  Datasources: Filter plugin errors to only show datasource plugins (#74339)
  Fix sticky header issue (#75710)
  Transformations: Extended support for variables in filter by name (#75734)
  Alerting: Fix being redirected to list view when clicking Save rule button (#75510)
  Tracing: Standardize on otel tracing (#75528)
  Fix developer links and newly discovered spelling errors (#75875)
  i18n: Mark up GeneralSettings for translations (#75827)
  DockedMegaMenu: Refactor and rename to simplify (#75872)
  sql: numeric inputs: use it's own simple implementation (#74904)
@oscarkilhed
Copy link
Contributor Author

This should also alleviate some issues reported by users in #25469

@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

@imatwawana imatwawana imatwawana left review comments

@mdvictor mdvictor mdvictor approved these changes

@grafanabot grafanabot Awaiting requested review from grafanabot grafanabot is a code owner

@ashharrison90 ashharrison90 Awaiting requested review from ashharrison90 ashharrison90 was automatically assigned from grafana/grafana-frontend-platform

@JoaoSilvaGrafana JoaoSilvaGrafana Awaiting requested review from JoaoSilvaGrafana JoaoSilvaGrafana was automatically assigned from grafana/grafana-frontend-platform

@leventebalogh leventebalogh Awaiting requested review from leventebalogh leventebalogh was automatically assigned from grafana/plugins-platform-frontend

@codeincarnate codeincarnate Awaiting requested review from codeincarnate

@baldm0mma baldm0mma Awaiting requested review from baldm0mma baldm0mma is a code owner

Assignees
No one assigned
Labels
add to changelog area/frontend area/transformations no-backport Skip backport of PR type/docs Flags the technical writing team for documentation support; auto adds to org-wide docs project
Projects
None yet
Milestone
10.2.0
Development

Successfully merging this pull request may close these issues.

Feature: Feed data into the "Organize Fields" transform from a dashboard variable
4 participants
@oscarkilhed @mdvictor @imatwawana @zerok