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

[Lens] Query based annotations #138753

Merged
merged 140 commits into from
Sep 8, 2022
Merged

[Lens] Query based annotations #138753

merged 140 commits into from
Sep 8, 2022

Conversation

dej611
Copy link
Contributor

@dej611 dej611 commented Aug 12, 2022

Summary

This PR contains:

  • integrating fetching query based annotations with Lens
  • the configuration side of the new query based annotation feature in Lens.

Query based annotations in the chart

Screenshot 2022-09-07 at 14 46 30

Screenshot 2022-09-07 at 14 48 49

Config panel

The Layer UI has changed to contain the dataView picker:

Screenshot 2022-08-12 at 17 49 44

Opening an annotation event will present a button group to switch to the Query based type:
Screenshot 2022-08-12 at 17 49 53

Once selected some additional features will be enabled in the panel to query/enrich the annotation/visualization:
Screenshot 2022-09-07 at 15 42 02

On dataview switch all fields selected are validated against the new dataview and errors are shown to the user in the main workspace:
Screenshot 2022-09-07 at 12 43 28

The query annotations are not fetched yet as it requires some more work to collect all the annotations expressions in a single go rather than per layer.

Tasks

  • Add logic to handle the dataview picker in the Annotation Layer
    • Move the dataview picker to the shared folder
  • Move the field picker to the shared folder
    • Move the Drag and drop code into the shared folder
    • Add it to the Text decoration section
    • Add it to the Tooltip section
  • Stub query_annotation expression
  • Handle dataViews references for visualizations
    • Add an extraction handle
    • Add an injection handle
  • Add a migration for the annotation type prop
    • Add migration for default indexPatternId when visualizations are using annotations (use first datasource dataview id?)
  • Tests
    • Fix existing tests
    • Add more unit tests
    • Add functional tests (will do as follow up with final feature)
  • Open in Lens from TSVB (will do as follow up)

Checklist

Delete any items that are not applicable to this PR.

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk Probability Severity Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space. Low High Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. High Low Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled. Medium High Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

dej611 added 30 commits June 20, 2022 18:40
@mbondyra mbondyra removed the request for review from a team September 7, 2022 12:40
@mbondyra mbondyra changed the title [Lens] Query based annotation UI configuration panel [Lens] Query based annotations Sep 7, 2022
Copy link
Contributor

@mbondyra mbondyra left a comment

Choose a reason for hiding this comment

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

Tested and reviewed (and of course, I put some lines here so hopefully we'll get some other pairs of eyes on it )

@flash1293
Copy link
Contributor

Both config UI and chart crash with a weird error if the data view is not available:
Screenshot 2022-09-08 at 10 00 23

Can also be fixed in a follow-up

showTimeSelect
selected={value}
onChange={onChange}
dateFormat="MMM D, YYYY @ HH:mm:ss.SSS"
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to use the dateFormat advanced setting here instead of hardcoding a format

Copy link
Contributor

Choose a reason for hiding this comment

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

Opened a follow-up issue for that

@flash1293
Copy link
Contributor

This works really well! The only two things we should still fix on this PR before merging is the migration issue (Marco already working on it) and the "missing data view" issue. @dej611 could you ping me once that works?

@dej611
Copy link
Contributor Author

dej611 commented Sep 8, 2022

Both issues have been resolves with the same fix, which fallbacks to the first dataView id found in references if none is available for the annotation layer.

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

LGTM - no further bugs found

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
expressionXY 144 145 +1
lens 898 905 +7
total +8

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
data 2441 2444 +3
eventAnnotation 163 170 +7
lens 532 550 +18
total +28

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
eventAnnotation 1.5KB 2.1KB +542.0B
expressionXY 116.0KB 116.9KB +977.0B
lens 1.2MB 1.2MB +15.2KB
total +16.7KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
eventAnnotation 5 3 -2
expressionXY 11 9 -2
total -4

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
data 430.6KB 430.7KB +122.0B
eventAnnotation 17.7KB 18.7KB +1006.0B
expressionXY 35.1KB 35.6KB +477.0B
total +1.6KB
Unknown metric groups

API count

id before after diff
data 3131 3144 +13
eventAnnotation 163 170 +7
expressionXY 152 153 +1
lens 618 639 +21
total +42

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@dej611 dej611 merged commit 1a1159b into elastic:main Sep 8, 2022
guskovaue pushed a commit to guskovaue/kibana that referenced this pull request Sep 12, 2022
* ⚗️ Initial code for query based annotations

* 🐛 Solved more conflicts

* ⚗️ More scaffolding layout

* ⚗️ Initial indexpatetrn move into frame

* ⚗️ Make field selection work

* 🚧 Fixed almost all dataViews occurrencies, but changeIndexPattern

* 🚧 More work on change index pattern

* Move lens dataViews state into main state

* 🔥 Remove some old cruft from the code

* 🐛 Fix dataViews layer change

* 🐛 Fix datasourceLayers refs

* 🔥 Remove more old cruft

* 🐛 Fix bug when loading SO

* 🐛 Fix initial existence flag

* 🏷️ Fix type issues

* 🏷️ Fix types and tests

* 🏷️ Fix types issues

* ✅ Fix more tests

* ✅ Fix with new dataViews structure

* ✅ Fix more test mocks

* ✅ More tests fixed

* 🔥 Removed unused prop

* ✅ Down to single broken test suite

* 🏷️ Fix type issue

* 👌 Integrate selector feedback

* ✅ Fix remaining unit tests

* 🏷️ fix type issues

* 🐛 Fix bug when creating dataview in place

* ✨ Update with latest dataview state + fix dataviews picker for annotations

* 🐛 Fix edit + remove field flow

* Update x-pack/plugins/lens/public/visualizations/xy/types.ts

* 📸 Fix snapshot

* 🐛 Fix the dataViews switch bug

* 🔥 remove old cruft

* ♻️ Revert removal from dataviews state branch

* ♻️ Load all at once

* 🔧 working on persistent state + fix new layer bug

* 🔥 remove unused stuff

* 🏷️ Fix some typings

* 🔧 Fix expression issue

* ✅ Add service unit tests

* 👌 Integrated feedback

* ✨ Add migration code for manual annotations

* 🏷️ Fix type issue

* ✅ Add some other unit test

* 🏷️ Fix more type issues

* 🐛 Fix importing issue

* ♻️ Make range default color dependant on opint one

* 🐛 Fix duplicate fields selection in tooltip section

* ✅ Add more unit tests

* ✅ Fix broken test

* 🏷️ Mute ts error for now

* ✅ Fix tests

* 🔥 Reduce plugin weight

* 🐛 prevent layout shift on panel open

* 🐛 Fix extract + inject visualization references

* 🏷️ fix type issues

* ✨ Add dataview reference migration for annotations

* 🔧 Add migration to embedadble

* 🏷️ Fix type export

* 🐛 Fix more conflicts with main

* ✅ Fix tests

* 🏷️ Make textField optional

* ♻️ Refactor query input to be a shared component

* 🐛 Fix missing import

* 🐛 fix more import issues

* 🔥 remove duplicate code

* 🐛 Fix dataView switch bug

* 🏷️ Fix type issue

* annotations with fetching_event_annotations

* portal for kql input fix

* timeField goes for default if not filled

* limit changes

* handle ad-hoc data view references correctly

* fix types

* adjust tests to datatable format (remove isHidden tests as it's filtered before)

* small refactors

* fix loading on dashboard

* empty is invalid (?) tbd

* new tooltip

* emptyDatatable

* ♻️ Flip field + query inputs

* 🏷️ Fix type issue

* ✨ Add field validation for text and tooltip fields

* tooltip for single annotation

* fix tests

* fix for non--timefilter dataview

* fix annotations test - the cause was that we now don't display label for aggregated annotations ever

* use eui elements

* newline problem solved

* ✅ Add more error tests

* 👌 Rename migration state version type

* fix types for expression chart

* 🐛 Fix i18n id

* 🏷️ Fix type issue

* fix hidden all annotations

* ✅ Fix tests after ishidden removal

* 🐛 Revert references migration to an in app solution

Co-authored-by: Joe Reuter <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Marta Bondyra <[email protected]>
Co-authored-by: Marta Bondyra <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Lens release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants