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

Correlations: Add an editor in Explore #73315

Merged
merged 89 commits into from
Oct 3, 2023
Merged

Conversation

gelicia
Copy link
Contributor

@gelicia gelicia commented Aug 15, 2023

What is this feature?

This is an addition to the Correlations project that allows people to create correlations from within Explore. The Correlations Admin page will still exist for editing, deleting and creating correlations with transformations, but this feature allows users to create correlations in a way that more closely maps to how they will use Correlations.

This introduces a new state in Explore - the correlations editor mode. In this mode, the datasource results contain data links attached to each field it returns. Clicking one of these links opens a right pane with a new component called the Correlations Helper. This helper contains a list of all variables a query can use. Saving creates a correlation between the left pane and the right pane, with the interpolation of any variables used.

Why do we need this feature?

The release of the Correlations feature came with one large critique - the feature was very abstract, and the editor created was hard to understand and use. This editor is an attempt to partially recify the ease of creating correlations.

Who is this feature for?

This feature is for users who would want to use the correlations feature but found the previous editor too obtuse to use.

Which issue(s) does this PR fix?:

Fixes #70684

Special notes for your reviewer:

Internal only: list of state tests/and history https://docs.google.com/document/d/1oyInlJBO0W_kCqvU4nVnMtMrcZqKfZoXGSi4Ltm38Xk/edit

This was removed after feedback from the frontend platform team. Frontend platform team
This adds a new optional property to breadcrumbs called onClick. This allows breadcrumbs to trigger an 'onClick' event instead of being a href to another page. For this feature, there is a full page editor mode for Explore, and feedback from the UX team indicated having this separate mode be a new breadcrumb would both help indicate the full page nature of the mode and additionally allow users to navigate out of the mode by clicking the "Explore" parent to the new mode. Exiting this mode is best done by a series of state changes rather than a navigation change.

Regarding the save data prompts
This iteration of this feature has three distinct ways to prompt people to save if an action could cause them to lose any correlations being created.

Navigating away from the page with a refresh or hitting the back button is one scenario, and requires using the useBeforeUnload hook. This looks like the following, using the browser native modal
Screenshot 2023-08-29 at 2 44 44 PM

Navigating to another page in Grafana requires using the react router Prompt component to stop the routing before it happens. This looks like the following:
Screenshot 2023-08-29 at 2 44 38 PM

Navigating within Explore (for example, hitting Exit Correlation Editor or the Explore breadcrumb) is much easier, and lets us use a fancier modal which looks like this
Screenshot 2023-08-29 at 2 44 53 PM

I would like to have all navigation outside of refreshing/back button look the same at some point, but would like to explore that outside of this PR

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.

@github-actions github-actions bot added the levitate breaking change A label indicating a breaking change and assigned by Levitate. label Aug 16, 2023
@grafana grafana deleted a comment from ephemeral-instances-bot bot Sep 29, 2023
@grafana grafana deleted a comment from ephemeral-instances-bot bot Sep 29, 2023
@gelicia
Copy link
Contributor Author

gelicia commented Sep 29, 2023

/deploy-to-hg

@ephemeral-instances-bot
Copy link

  • Preparing your instance. A comment containing your instance's url will be added to this PR when the instance is ready.
  • Your instance will be ready in ~10 minutes.
  • Check the GitHub actions tab to follow the workflow progress
  • Slack channel: #proj-ephemeral-hg-instances
  • Building instance with kristina/correlations-editor oss branch and main enterprise branch. How to choose a branch

@ephemeral-instances-bot
Copy link

@diegoadams
Copy link

With more content to scroll the bar disappears after scrolling down.

Screen.Recording.2023-09-29.at.14.33.52.mov
As discussed @gelicia will check if it's easy to fix. @diegoadams do you think it's a blocker?

Ignore if this has already been addressed, but I think it is important that the blue section remains fixed considering this is a new environment for the user. We just need them to be aware at all times that they are in the edit mode.

@gelicia
Copy link
Contributor Author

gelicia commented Oct 2, 2023

@diegoadams @ifrost fixed the scrollbar by adding a height offset if applicable
https://github.com/grafana/grafana/assets/1533128/7cb59fb5-b6c6-4ea7-ac22-ac6d234202be

Copy link
Contributor

@ifrost ifrost left a comment

Choose a reason for hiding this comment

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

…na/correlations-editor

# Conflicts:
#	public/app/features/explore/Explore.tsx
@gelicia gelicia merged commit 4b3d63d into main Oct 3, 2023
19 checks passed
@gelicia gelicia deleted the kristina/correlations-editor branch October 3, 2023 14:28
bergquist added a commit that referenced this pull request Oct 3, 2023
* 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)
mildwonkey pushed a commit that referenced this pull request Oct 4, 2023
* Start adding correlations editor mode

* Add selector for merge conflict resolution

* Enable saving

* Build out new corelation helper component

* flesh out save with label/description, change color

* Have breadcrumb exit correlation editor mode

* Add extension property to show/hide, use it for correlations

* Bring in feature toggle

* Remove unnecessary param

* Cleanup

* Parse logs json

* Work on correlation edit mode bar

* Tinker with a top element for the editor mode

* Handle various explore state changes with correlations editor mode

* WIP - add unsaved changes modal

* Have correlation bar always rendered, sometimes hidden

* Add various prompt modals

* Clear correlations data on mode bar unmount, only use not left pane changes to count as dirty

* Move special logic to explore

* Remove all shouldShow logic from plugin extensions

* remove grafana data changes

* WIP - clean up correlations state

* Interpolate data before sending to onclick

* Override outline button coloring to account for dark background

* More cleanup, more color tweaking

* Prettier formatting, change state to refer to editor

* Fix tests

* More state change tweaks

* ensure correlation save ability, change correlation editor state vars

* fix import

* Remove independent selector for editorMode, work close pane into editor exit flow

* Add change datasource post action

* Clean up based on PR feedback, handle closing left panel with helper better

* Remove breadcrumb additions, add section and better ID to cmd palette action

* Interpolate query results if it is ran with a helper with vars

* Pass the datasource query along with the correlate link to ensure the datasource unique ID requirement passes

* Use different onmount function to capture state of panes at time of close instead of time of mount

* Fix node graph’s datalink not working

* Actually commit the fix to saving

* Fix saving correlations with mixed datasource to use the first query correlation UID

* Add tracking

* Use query datasources in mixed scenario, move exit tracking to click handler

* Add correlations to a place where both can be used in the correlations editor

* Be more selective on when we await the datasource get

* Fix CSS to use objects

* Update betterer

* Add test around new decorator functionality

* Add tests for decorate with correlations

* Some reorganization and a few tweaks based on feedback

* Move dirty state change to state function and out of component

* Change the verbiage around a little

* Various suggestions from Gio

Co-authored-by: Giordano Ricci <me@giordanoricci.com>

* More small Gio-related tweaks

* Tie helper data to datasource - clear it out when the datasource changes

* Missed another Gio tweak

* Fix linter error

* Only clear helper data on left pane changes

* Add height offset for correlation editor bar so it doesn’t scroll off page

---------

Co-authored-by: Piotr Jamróz <pm.jamroz@gmail.com>
Co-authored-by: Giordano Ricci <me@giordanoricci.com>
@zerok zerok modified the milestones: 10.2.x, 10.2.0 Oct 23, 2023
bohandley added a commit that referenced this pull request Jan 27, 2024
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.

Glue: Create correlations directly from Explore
7 participants