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

[Maps] Enable all zoom levels for all users #96093

Merged
merged 11 commits into from
Apr 5, 2021

Conversation

thomasneirynck
Copy link
Contributor

@thomasneirynck thomasneirynck commented Apr 1, 2021

Summary

Enables all EMS zoom levels for all users. Do this by default by setting license=sspl in the request for the old OSS visualizations (region, coordinate, and vega).

In practice, with this PR, Kibana-requests to EMS follow this approach:

  • Maps-app will continue to submit the actual license-key
  • Vega, coordinate, and region maps will use license=sspl, regardless of distribution. Users of these visualizations will get all zoom levels, regardless of distribution.

Apart from the end-user benefits, there are some internal benefits:

  • allows us remove the entire mapsLicensingPlugin. Because of this, mapsEms-plugin does not need to be loaded on page-load, cutting out all the unnecessary ems-loading. This will likely fix [Maps] maps_legacy async bundle is loaded on startup #92664.
  • remove the setQueryParams API method from ServiceSettings, as license-injection by x-pack code is no longer necessary.

This also sets us up for other simplifciations:

For maintainers

@thomasneirynck thomasneirynck added the WIP Work in progress label Apr 1, 2021
@nyurik
Copy link
Contributor

nyurik commented Apr 1, 2021

tile server license=sspl support is now in production

@thomasneirynck thomasneirynck removed the WIP Work in progress label Apr 2, 2021
@thomasneirynck thomasneirynck marked this pull request as ready for review April 2, 2021 14:46
@thomasneirynck thomasneirynck requested a review from a team as a code owner April 2, 2021 14:46
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis (Team:Geo)

Copy link
Contributor

@tylersmalley tylersmalley left a comment

Choose a reason for hiding this comment

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

Limits change LGTM, especially when it means getting rid of things!

Copy link
Member

@nickpeihl nickpeihl left a comment

Choose a reason for hiding this comment

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

lgtm!

tested in Firefox and code review

@@ -45,6 +43,9 @@ export class ServiceSettings implements IServiceSettings {
return fetch(...args);
},
});
// any kibana user, regardless of distribution, should get all zoom levels
// use `sspl` license to indicate this
this._emsClient.addQueryParams({ license: 'sspl' });
Copy link
Member

Choose a reason for hiding this comment

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

Maybe out of scope for this PR, but should we consider adding a license parameter to the EMSClient constructor in the @elastic/ems-client library?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, let's do that in separate, since it requires upstream change. I can create a separate PR for the ems-client for this.

@thomasneirynck
Copy link
Contributor Author

@elasticmachine merge upstream

@thomasneirynck thomasneirynck merged commit 8e11e25 into elastic:master Apr 5, 2021
thomasneirynck added a commit to thomasneirynck/kibana that referenced this pull request Apr 5, 2021
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
mapsLegacy 50 49 -1
mapsLegacyLicensing 4 - -4
total -5

Async chunks

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

id before after diff
mapsEms 223.1KB 222.9KB -227.0B
mapsLegacy 281.1KB 278.0KB -3.1KB
total -3.4KB

Page load bundle

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

id before after diff
mapsLegacy 64.3KB 62.9KB -1.3KB
mapsLegacyLicensing 2.8KB - -2.8KB
total -4.2KB

History

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

thomasneirynck added a commit that referenced this pull request Apr 5, 2021
jloleysens added a commit to jloleysens/kibana that referenced this pull request Apr 6, 2021
…-nav

* 'master' of github.com:elastic/kibana: (106 commits)
  [Lens] don't use eui variables for zindex (elastic#96117)
  Remove /src/legacy (elastic#95510)
  skip flaky suite (elastic#95899)
  [Dashboard] Fix Lens and TSVB chart tooltip positioning relative to global headers (elastic#94247)
  fixes a skipped management x-pack test (elastic#96178)
  [App Search] API logs: Add log detail flyout (elastic#96162)
  [tech-debt] Remove defunct opacity parameters from EUI shadow functions (elastic#96191)
  Add Input Controls project configuration (elastic#96238)
  [file upload] document file upload privileges and provide actionable UI when failures occur (elastic#95883)
  Revert "TS Incremental build exclude test files (elastic#95610)" (elastic#96223)
  [App Search] Added Sample Response section to Result Settings (elastic#95971)
  [Maps] Safe-erase text-field (elastic#94873)
  [RAC][Alert Triage][TGrid] Update the Alerts Table (TGrid) API to implement `renderCellValue` (elastic#96098)
  [Maps] Enable all zoom levels for all users (elastic#96093)
  Use plugin version in its publicPath (elastic#95945)
  [Enterprise Search] Expose core.chrome.setIsVisible for use in Workplace Search (elastic#95984)
  [Workplace Search] Add sub nav and fix rendering bugs in Personal dashboard (elastic#96100)
  [OBS]home page is showing incorrect value of APM throughput (tpm) (elastic#95991)
  [Observability] Exploratory View initial skeleton (elastic#94426)
  [KQL] Fixed styles of KQL textarea for the K8 theme (elastic#96190)
  ...

# Conflicts:
#	x-pack/plugins/snapshot_restore/__jest__/client_integration/helpers/restore_snapshot.helpers.ts
@rashmivkulkarni rashmivkulkarni added release_note:skip Skip the PR/issue when compiling release notes and removed release_note:enhancement labels Apr 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation release_note:skip Skip the PR/issue when compiling release notes v7.13.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Maps] maps_legacy async bundle is loaded on startup
8 participants