Skip to content

Replace legacy Maps embeddable with MapComponent in APM GeoMap#182753

Merged
nickpeihl merged 9 commits intoelastic:mainfrom
nickpeihl:decouple-map-embeddable-apm
May 9, 2024
Merged

Replace legacy Maps embeddable with MapComponent in APM GeoMap#182753
nickpeihl merged 9 commits intoelastic:mainfrom
nickpeihl:decouple-map-embeddable-apm

Conversation

@nickpeihl
Copy link
Copy Markdown
Contributor

@nickpeihl nickpeihl commented May 6, 2024

Summary

Replaces the Maps embeddable with a new MapComponent in the APM solution.

The Maps plugin now provides an easier to use MapComponent for consumers. The legacy MapEmbeddable factory is also being removed as part of #174960 which requires making changes to consumers.

For maintainers

@nickpeihl nickpeihl changed the title Decouple Maps embeddable from Observability Remove legacy Maps embeddable from Observability GeoMap May 7, 2024
@nickpeihl nickpeihl changed the title Remove legacy Maps embeddable from Observability GeoMap Place legacy Maps embeddable with MapComponent in Observability GeoMap May 7, 2024
@nickpeihl nickpeihl changed the title Place legacy Maps embeddable with MapComponent in Observability GeoMap Replace legacy Maps embeddable with MapComponent in Observability GeoMap May 7, 2024
@nickpeihl nickpeihl changed the title Replace legacy Maps embeddable with MapComponent in Observability GeoMap Replace legacy Maps embeddable with MapComponent in APM GeoMap May 7, 2024
@nickpeihl nickpeihl added Team:obs-ux-infra_services - DEPRECATED DEPRECATED - Use Team:obs-presentation. Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// release_note:skip Skip the PR/issue when compiling release notes v8.15.0 project:embeddableRebuild labels May 7, 2024
@nickpeihl nickpeihl marked this pull request as ready for review May 7, 2024 12:16
@nickpeihl nickpeihl requested a review from a team May 7, 2024 12:16
@elasticmachine
Copy link
Copy Markdown
Contributor

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

@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

@nickpeihl
Copy link
Copy Markdown
Contributor Author

/ci

@botelastic botelastic bot added the ci:project-deploy-observability Create an Observability project label May 7, 2024
nickpeihl added 2 commits May 7, 2024 15:22
The MapComponent already provides the basemap to the embeddable so there is no need to await the basemap descriptor
</>
<div
data-test-subj="serviceOverviewEmbeddedMap"
style={{ width: '100%', height: '500px', display: 'flex', flex: '1 1 100%', zIndex: 1 }}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Using style instead of the emotion css method resolves the issue that also affects the main branch where the height of this div may change to 0 when the filters change.

mobile-apm-before.mp4
mobile-apm-after.mp4

query: `${PROCESSOR_EVENT}:${ProcessorEvent.span} and ${SPAN_SUBTYPE}:${MobileSpanSubtype.Http} and ${SPAN_TYPE}:${MobileSpanType.External}`,
};

const basemapLayerDescriptor = await maps?.createLayerDescriptors?.createBasemapLayerDescriptor();
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's not necessary to add the basemapLayerDescriptor as the MapComponent already takes care of that.

@@ -34,137 +29,41 @@ function EmbeddedMapComponent({
filters: Filter[];
dataView?: DataView;
Copy link
Copy Markdown
Contributor Author

@nickpeihl nickpeihl May 8, 2024

Choose a reason for hiding this comment

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

The dataView passed into this component is an ad-hoc data view created from the useAdHocApmDataView hook. However, the ad-hoc dataview is created without specifying a timeFieldName. So Maps does not think the source is time aware and will not respond to changes to the time picker. This is a bug that currently exists in main as well.

The useAdHocApmDataView hook is used in multiple places in this plugin so I did not feel comfortable making any changes there.

Copy link
Copy Markdown
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

LGTM
code review only

@kibana-ci
Copy link
Copy Markdown

kibana-ci commented May 9, 2024

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
apm 3.2MB 3.2MB -1.6KB

Canvas Sharable Runtime

The Canvas "shareable runtime" is an bundle produced to enable running Canvas workpads outside of Kibana. This bundle is included in third-party webpages that embed canvas and therefor should be as slim as possible.

id before after diff
module count - 5407 +5407
total size - 8.8MB +8.8MB
Unknown metric groups

ESLint disabled line counts

id before after diff
apm 73 72 -1

Total ESLint disabled count

id before after diff
apm 87 86 -1

History

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

Copy link
Copy Markdown
Contributor

@cauemarcondes cauemarcondes left a comment

Choose a reason for hiding this comment

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

LGTM

@nickpeihl nickpeihl merged commit 99e7bd3 into elastic:main May 9, 2024
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting ci:project-deploy-observability Create an Observability project project:embeddableRebuild release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-infra_services - DEPRECATED DEPRECATED - Use Team:obs-presentation. Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v8.15.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants