Correlations: Add transformations to Explore Editor #75930

gelicia · 2023-10-03T21:59:10Z

What is this feature?

Adds the ability to create one or more correlation transformations from the Explore correlation editor.

Why do we need this feature?

Transformations are almost necessary for creating correlations on logs. Logs' data format generally has more than one piece of information per field, notably the log line field and the labels field. Transformations were created to parse out information people may want, via a regular expression or logfmt.

Who is this feature for?

People who want to create correlations where data may be within in one field amongst other data.

Which issue(s) does this PR fix?:

Fixes #75832

Special notes for your reviewer:
Testing this with devenv, using Loki is probably your best bet. Logfmt will parse the devenv line, and labels can be parsed out with the following regex expressions:

namespace":"([a-z]*) - this is an unnamed capture group
namespace":"(?<test2>[a-z]*) this is a named capture group

In the ephemeral, loki data is available in the grafanacloud-ephemeral1511182175930gelicia-usage-insights Loki datasource. Doing a logfmt transformation against a Line field adds a lot of variables. Adding a regex transformation on labels works with the following

instance_type":"([a-z]*) - this is an unnamed capture group
instance_type":"(?<name>[a-z]*) - this is a named capture group

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 · 2023-10-03T22:23:44Z

Backend code coverage report for PR #75930
No changes

github-actions · 2023-10-03T22:23:45Z

Frontend code coverage report for PR #75930

Plugin	Main	PR	Difference
correlations	90.15%	89.11%	-1.04%
explore	85.57%	84.42%	-1.15%

…na/correlations-editor-transform

…er (WIP)

…na/correlations-editor-transform

public/app/features/explore/correlationEditLogic.test.ts

ifrost

Transformations vars are not interpolated inside the editor

After adding transformations I was not able to use vars inside the editor / right pane. After saving everything works fine. It looks like only when we're inside the editor, then transformation vars are not used when interpolating.

interpolation.mov

Also probably not related to this PR but worth checking:

Mixed data source is used inside the default label instead of the underlying data source

Regex is case insenstive

I think it's been like this since we added this but I wonder if it shouldn't base case sensitive to allow capturing more specific cases. Or at least at it to the tooltip / docs.

ifrost · 2023-10-23T09:32:39Z

public/app/features/explore/correlationEditLogic.test.ts

+    ${CORRELATION_EDITOR_POST_CONFIRM_ACTION.CLOSE_PANE}        | ${true}  | ${true}  | ${false} | ${'Closing the pane will cause the correlation in progress to be lost. Would you like to save before continuing?'}
+    ${CORRELATION_EDITOR_POST_CONFIRM_ACTION.CLOSE_PANE}        | ${true}  | ${false} | ${true}  | ${'Closing the pane will cause the query in the right pane to be re-ran and links added to that data. Would you like to save before continuing?'}
+    ${CORRELATION_EDITOR_POST_CONFIRM_ACTION.CLOSE_PANE}        | ${true}  | ${true}  | ${true}  | ${'Closing the pane will cause the correlation in progress to be lost. Would you like to save before continuing?'}
+    ${CORRELATION_EDITOR_POST_CONFIRM_ACTION.CHANGE_DATASOURCE} | ${false} | ${false} | ${false} | ${undefined}


It may not happen with each data source but sometimes changing the data source without making any changes causes the modal to appear. Could it be because the editor calls onChange?

change-ds-triggers-the-prompt.mov

Yeah :/ and I'm not quite sure what the solution is.

The order of events is this

User changes the datasource to cloudwatch. This completes. The onChange function that we use to track a dirty query is passed into the query editor

Cloudwatch's query editor internally runs a function that calls the onChange function passed in https://github.com/grafana/grafana/blob/main/public/app/plugins/datasource/cloudwatch/components/QueryEditor/QueryEditor.tsx#L28 - If you look at the queries before and after the change, it sets up the data structure for CW with log groups, region, an empty logGroups array, etc.

The onChange function is executed and the query gets marked as dirty even without any user interaction interacting with the editor.

I need to investigate some more, but I don't know how to make the distinction between a user performed query change and an automatic one, when the editor itself doesn't make that distinction at all. I thought that ensuring the query was not marked as dirty after the DS changed would be enough, but the query editor for the plugin is mounted after and I don't think there's any sort of state change that happens after that mount occurs.

I feel like this is sorta similar to #52213 in that both situations would be very much improved if we knew when the query editor was done doing whatever it needed to do.

public/app/features/explore/correlationEditLogic.test.ts

ifrost · 2023-10-23T09:38:02Z

public/app/features/explore/correlationEditLogic.test.ts

+    ${CORRELATION_EDITOR_POST_CONFIRM_ACTION.CHANGE_DATASOURCE} | ${true}  | ${true}  | ${true}  | ${'Changing the datasource will cause the correlation in progress to be lost. Would you like to save before continuing?'}
+    ${CORRELATION_EDITOR_POST_CONFIRM_ACTION.CLOSE_EDITOR}      | ${false} | ${false} | ${false} | ${undefined}
+    ${CORRELATION_EDITOR_POST_CONFIRM_ACTION.CLOSE_EDITOR}      | ${false} | ${true}  | ${false} | ${'Closing the editor will cause the correlation in progress to be lost. Would you like to save before continuing?'}
+    ${CORRELATION_EDITOR_POST_CONFIRM_ACTION.CLOSE_EDITOR}      | ${false} | ${false} | ${true}  | ${'Closing the editor will lose the changed query. Would you like to save before continuing?'}


In what sense "the query is lost"? I tried closing the right pane after changing the query and it seems to run exactly the same query without losing anything:

losing-the-query.mov

Ah this is true. I might change this to say "Closing the editor will remove the variables, and your changed query may no longer be valid. Would you like to save before continuing?" in this instance

Fixed!

ifrost · 2023-10-23T09:39:52Z

public/app/features/explore/correlationEditLogic.test.ts

+  it.each`
+    action                                                      | isLeft   | dirCor   | dirQuer  | expected
+    ${CORRELATION_EDITOR_POST_CONFIRM_ACTION.CLOSE_PANE}        | ${false} | ${false} | ${false} | ${undefined}
+    ${CORRELATION_EDITOR_POST_CONFIRM_ACTION.CLOSE_PANE}        | ${false} | ${true}  | ${false} | ${'Closing the pane will cause the correlation in progress to be lost. Would you like to save before continuing?'}


Changing the label doesn't mark a correlation as dirty. Is it expected?

Screen.Recording.2023-10-23.at.11.38.28.mov

No - changing the label means the correlation dirty state is true. This was a regression when I used the default value as a default value instead of the default value being blank and only using the default value on save.

And actually I think I'll be able to tackle this and the mixed datasource problem at the same time, so I'll be looking into this more.

Fixed!

…na/correlations-editor-transform

gelicia · 2023-10-23T19:33:43Z

Transformations vars are not interpolated inside the editor

fixed!

Mixed data source is used inside the default label instead of the underlying data source

Fixed (finally...)

Regex is case insenstive

I did this on purpose to increase the likelihood that people will have matches and I'd like to keep it that way for now. I added it to the tooltip.

…na/correlations-editor-transform

ifrost

Is documentation being added as part of this PR?

ifrost · 2023-10-31T12:51:46Z

public/app/features/correlations/utils.ts

+  }
+};
+
+export const generateDefaultLabel = async (sourcePane: ExploreItemState, targetPane: ExploreItemState) => {


Ideally correlations utils should not rely on any Explore specific types. This function could get datasourceRef passed instead of entire ExploreItemState.

ifrost · 2023-10-31T12:55:32Z

public/app/features/explore/correlationEditLogic.test.ts

+    ${CORRELATION_EDITOR_POST_CONFIRM_ACTION.CHANGE_DATASOURCE} | ${false} | ${false} | ${true}  | ${'Changing the datasource will lose the changed query. Would you like to save before continuing?'}
+    ${CORRELATION_EDITOR_POST_CONFIRM_ACTION.CHANGE_DATASOURCE} | ${false} | ${true}  | ${true}  | ${'Changing the datasource will lose the changed query. Would you like to save before continuing?'}
+    ${CORRELATION_EDITOR_POST_CONFIRM_ACTION.CHANGE_DATASOURCE} | ${true}  | ${false} | ${false} | ${undefined}
+    ${CORRELATION_EDITOR_POST_CONFIRM_ACTION.CHANGE_DATASOURCE} | ${true}  | ${true}  | ${false} | ${'Changing the datasource will cause the correlation in progress to be lost. Would you like to save before continuing?'}


Personally I find it confusing that I'm asked if I want to save the correlation at this point. I'd expect a simple question like "Your progress will be lost. Do you want to continue?". But I'll leave it up to UX.

gelicia · 2023-10-31T16:41:05Z

Is documentation being added as part of this PR?

Created #77457 and that will be my next task

…na/correlations-editor-transform

* Add transformation add modal and use it * Hook up saving * Add transformation vars to var list, show added transformations * Form validation * Remove type assertion, start building out transformation data in helper (WIP) * Style expression better, add delete logic * Add ability to edit, additional styling on transformation card in helper * simplify styling, conditionally run edit set up logic * Keep more field information in function, integrate it with new editor * Show default label on collapsed section, use deleteButton for confirmation of deleting transformations * Change transformation add calculations from function to hook, add label to collapsed header, add transformation tooltip * Make correlation and editor dirty state distinctive and integrate, WIP * Track action pane for more detailed messaging/actions * Better cancel modal logic * Remove changes to adminsitration transformation editor * Remove debugging line * Remove unneeded comment * Add in logic for closing editor mode * Add tests for modal logic * Use state to build vars list for helper * WIP * Fix labels and dirty state * Fix bad message and stop exiting mode if discard action is performed * Fix tests * Update to not use unstable component and tweak default label

gelicia added 2 commits October 3, 2023 12:56

Add transformation add modal and use it

62e5db7

Hook up saving

6cf5c52

grafana-delivery-bot bot added this to the 10.2.x milestone Oct 3, 2023

grafana-pr-automation bot added area/frontend area/explore labels Oct 3, 2023

gelicia added add to changelog no-backport Skip backport of PR and removed area/explore area/frontend labels Oct 3, 2023

grafana-pr-automation bot added area/frontend area/explore labels Oct 3, 2023

Add transformation vars to var list, show added transformations

a9e6b07

gelicia force-pushed the kristina/correlations-editor-transform branch from bd6473d to a9e6b07 Compare October 4, 2023 01:20

gelicia added 9 commits October 4, 2023 09:50

Merge branch 'main' of https://github.com/grafana/grafana into kristi…

5185d53

…na/correlations-editor-transform

Form validation

67581da

Merge branch 'main' of https://github.com/grafana/grafana into kristi…

8f2196b

…na/correlations-editor-transform

Remove type assertion, start building out transformation data in help…

ca7b556

…er (WIP)

Merge branch 'main' of https://github.com/grafana/grafana into kristi…

da4adcd

…na/correlations-editor-transform

Style expression better, add delete logic

e3f1420

Add ability to edit, additional styling on transformation card in helper

a068dd9

Merge branch 'main' of https://github.com/grafana/grafana into kristi…

5e5dae5

…na/correlations-editor-transform

simplify styling, conditionally run edit set up logic

48b835d

grafana deleted a comment from ephemeral-instances-bot bot Oct 6, 2023

gelicia marked this pull request as ready for review October 6, 2023 17:37

gelicia requested review from a team as code owners October 6, 2023 17:37

gelicia removed the request for review from a team October 6, 2023 17:37

Add tests for modal logic

e54c6bc

gelicia requested a review from a team as a code owner October 20, 2023 15:51

gelicia requested review from mckn and removed request for a team October 20, 2023 15:51

gelicia commented Oct 20, 2023

View reviewed changes

public/app/features/explore/correlationEditLogic.test.ts Show resolved Hide resolved

ifrost reviewed Oct 23, 2023

View reviewed changes

Merge branch 'main' of https://github.com/grafana/grafana into kristi…

f4df45d

…na/correlations-editor-transform

Use state to build vars list for helper

f473dd6

gelicia force-pushed the kristina/correlations-editor-transform branch from 1533148 to f473dd6 Compare October 23, 2023 21:10

gelicia added 8 commits October 24, 2023 16:45

Merge branch 'main' of https://github.com/grafana/grafana into kristi…

c63e73f

…na/correlations-editor-transform

Merge branch 'main' of https://github.com/grafana/grafana into kristi…

8f959c2

…na/correlations-editor-transform

WIP

aa123d6

Fix labels and dirty state

f1fdd9c

Fix bad message and stop exiting mode if discard action is performed

4daf7cc

Fix tests

2d8ff0a

Merge branch 'main' of https://github.com/grafana/grafana into kristi…

eff1a8f

…na/correlations-editor-transform

Merge branch 'main' of https://github.com/grafana/grafana into kristi…

f9c93e1

…na/correlations-editor-transform

ifrost reviewed Oct 31, 2023

View reviewed changes

gelicia added 3 commits November 2, 2023 10:52

Merge branch 'main' of https://github.com/grafana/grafana into kristi…

e95d05b

…na/correlations-editor-transform

Update to not use unstable component and tweak default label

bfe4752

Merge branch 'main' of https://github.com/grafana/grafana into kristi…

2419a42

…na/correlations-editor-transform

gelicia merged commit 9e0ca0d into main Nov 2, 2023
18 checks passed

gelicia deleted the kristina/correlations-editor-transform branch November 2, 2023 20:09

ifrost mentioned this pull request Nov 6, 2023

i18n: Markup up correlations forms for translations #75264

Merged

gelicia mentioned this pull request Nov 7, 2023

Correlations: Support JSON Transform #77767

Open

aangelisc modified the milestones: 10.3.x, 10.2.3 Dec 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Correlations: Add transformations to Explore Editor #75930

Correlations: Add transformations to Explore Editor #75930

gelicia commented Oct 3, 2023 •

edited

Loading

github-actions bot commented Oct 3, 2023

github-actions bot commented Oct 3, 2023 •

edited

Loading

ifrost left a comment

ifrost Oct 23, 2023

gelicia Oct 26, 2023 •

edited

Loading

ifrost Oct 23, 2023

gelicia Oct 23, 2023 •

edited

Loading

ifrost Oct 23, 2023

gelicia Oct 23, 2023 •

edited

Loading

gelicia commented Oct 23, 2023 •

edited

Loading

ifrost left a comment

ifrost Oct 31, 2023

ifrost Oct 31, 2023

gelicia commented Oct 31, 2023

Correlations: Add transformations to Explore Editor #75930

Correlations: Add transformations to Explore Editor #75930

Conversation

gelicia commented Oct 3, 2023 • edited Loading

github-actions bot commented Oct 3, 2023

github-actions bot commented Oct 3, 2023 • edited Loading

ifrost left a comment

Choose a reason for hiding this comment

ifrost Oct 23, 2023

Choose a reason for hiding this comment

gelicia Oct 26, 2023 • edited Loading

Choose a reason for hiding this comment

ifrost Oct 23, 2023

Choose a reason for hiding this comment

gelicia Oct 23, 2023 • edited Loading

Choose a reason for hiding this comment

ifrost Oct 23, 2023

Choose a reason for hiding this comment

gelicia Oct 23, 2023 • edited Loading

Choose a reason for hiding this comment

gelicia commented Oct 23, 2023 • edited Loading

ifrost left a comment

Choose a reason for hiding this comment

ifrost Oct 31, 2023

Choose a reason for hiding this comment

ifrost Oct 31, 2023

Choose a reason for hiding this comment

gelicia commented Oct 31, 2023

gelicia commented Oct 3, 2023 •

edited

Loading

github-actions bot commented Oct 3, 2023 •

edited

Loading

gelicia Oct 26, 2023 •

edited

Loading

gelicia Oct 23, 2023 •

edited

Loading

gelicia Oct 23, 2023 •

edited

Loading

gelicia commented Oct 23, 2023 •

edited

Loading