Skip to content

[maps] time series geo line - #159267

Merged
nreese merged 42 commits into
elastic:mainfrom
nreese:time_series_geo_line
Jun 29, 2023
Merged

[maps] time series geo line#159267
nreese merged 42 commits into
elastic:mainfrom
nreese:time_series_geo_line

Conversation

@nreese

@nreese nreese commented Jun 7, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Part of #141978

PR updates tracks layer with "group by time series" logic. When true, geo_line metric aggregation is proceeded by time_series bucket aggregation instead of filters bucket aggregation (used by existing terms split).

UI when creating tracks layer with time series data view

Screen Shot 2023-06-22 at 12 35 46 PM Screen Shot 2023-06-22 at 12 35 55 PM Screen Shot 2023-06-22 at 12 49 22 PM

UI when editing tracks layer with time series data view

Screen Shot 2023-06-22 at 12 36 17 PM Screen Shot 2023-06-22 at 12 36 24 PM

Test instructions

  • clone https://github.com/thomasneirynck/faketracks
  • cd into faketracks
  • run npm install
  • run node ./generate_tracks.js --isTimeSeries
  • In Kibana, create tracks data view
  • In Maps, create new map and add Tracks layer. Select Tracks data view.

@nreese
nreese force-pushed the time_series_geo_line branch from ba45920 to eb01d93 Compare June 14, 2023 17:01
@nreese
nreese force-pushed the time_series_geo_line branch from 062dd17 to b162a33 Compare June 14, 2023 18:46
@nreese

nreese commented Jun 15, 2023

Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@nreese

nreese commented Jun 20, 2023

Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@nreese
nreese force-pushed the time_series_geo_line branch from 7c940eb to e556c4f Compare June 20, 2023 19:56
@nreese nreese added the v8.10.0 label Jun 20, 2023
@nreese
nreese force-pushed the time_series_geo_line branch from 6753fc3 to 193ea30 Compare June 20, 2023 23:14
@nreese
nreese marked this pull request as ready for review June 22, 2023 21:27
@nreese
nreese requested review from a team as code owners June 22, 2023 21:27
@nreese nreese added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// release_note:feature Makes this part of the condensed release notes Feature:Maps labels Jun 22, 2023
@elasticmachine

elasticmachine commented Jun 22, 2023

Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@nreese
nreese requested a review from gchaps June 22, 2023 21:44
jughosta
jughosta reviewed Jun 23, 2023
View reviewed changes
Comment thread src/plugins/data_views/common/types.ts
@nreese
nreese requested a review from jughosta June 23, 2023 13:00
jughosta
jughosta approved these changes Jun 23, 2023
View reviewed changes
Comment thread src/plugins/data_views/common/types.ts
@nreese

nreese commented Jun 23, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Since it can take time it might be worth adding | 'position' here (at least temporary) and run CI once to check that nothing breaks.

The problem with adding | 'position' is then we have to remember to remove it. ts-expect-error comments will fail tslint when no error is thrown so they are noisy and will force us to remove them. Also, I think its bad practice to keep multiple copies of the same concept in multiple places. Kibana should rely on elasticsearch types defined from elasticsearch-specification and not provide workarounds that just become tech debt

@nickpeihl
nickpeihl self-requested a review June 23, 2023 17:12
nickpeihl
nickpeihl approved these changes Jun 26, 2023
View reviewed changes

@nickpeihl nickpeihl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Couple of nits, but otherwise lgtm!

Love to see us at the cutting edge of adopting time series and geoline simplification!

Comment thread x-pack/plugins/maps/public/classes/sources/es_geo_line_source/convert_to_geojson.ts
Comment thread x-pack/plugins/maps/public/classes/sources/es_geo_line_source/es_geo_line_source.tsx
Comment thread x-pack/plugins/maps/public/classes/sources/es_geo_line_source/es_geo_line_source.tsx Outdated
Comment thread x-pack/plugins/maps/public/classes/sources/es_geo_line_source/update_source_editor.tsx
Comment thread x-pack/plugins/maps/public/classes/sources/es_geo_line_source/geo_line_form/size_slider.tsx
Comment thread x-pack/plugins/maps/public/classes/sources/es_geo_line_source/geo_line_form/i18n_constants.ts
@nreese
nreese requested review from amyjtechwriter and removed request for gchaps June 28, 2023 16:15
@nreese

nreese commented Jun 28, 2023

Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@amyjtechwriter amyjtechwriter added the ui-copy Review of UI copy with docs team is recommended label Jun 29, 2023
@amyjtechwriter amyjtechwriter mentioned this pull request Jun 29, 2023
Closed
amyjtechwriter
amyjtechwriter approved these changes Jun 29, 2023
View reviewed changes

@amyjtechwriter amyjtechwriter left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM copy side, and thank you for including testing instructions.

@nreese

nreese commented Jun 29, 2023

Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@kibana-ci

kibana-ci commented Jun 29, 2023

Copy link
Copy Markdown

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
maps 987 993 +6

Async chunks

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

id before after diff
maps 2.7MB 2.7MB +6.6KB
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 14 16 +2
securitySolution 413 417 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 15 17 +2
securitySolution 492 496 +4
total +6

History

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

@nreese
nreese merged commit 7340007 into elastic:main Jun 29, 2023
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickpeihl nickpeihl nickpeihl approved these changes
+2 more reviewers
@jughosta jughosta jughosta approved these changes
@amyjtechwriter amyjtechwriter amyjtechwriter approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

backport:skip This PR does not require backporting Feature:Maps release_note:feature Makes this part of the condensed release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// ui-copy Review of UI copy with docs team is recommended v8.10.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@nreese @elasticmachine @kibana-ci @jughosta @nickpeihl @amyjtechwriter @kibanamachine