[APM] License prompt for service map #52668

smith · 2019-12-10T19:13:14Z

Display a link to platinum license upgrade on the service map.

Also add useKibanaUrl and useLicense hooks.

Make the LicenseContext (which is used in a couple class components and on page load) use the license observable from the NP plugin.

Add missing export of useObservable to kibana_react.

Closes #52375.

sorenlouv · 2019-12-12T22:46:45Z

x-pack/legacy/plugins/apm/public/hooks/useLicense.ts

+      setLicense(nextLicense);
+    });
+    return () => subscription.unsubscribe();
+  }, [license$]);


Shouldn't the subscription happen inside the context and not the other way around? If several components call useLicense we'll have several subscriptions. If we add it to the context we'll only have one.

Fixed in b7e5f18.

sorenlouv · 2019-12-12T22:48:23Z

x-pack/legacy/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/index.tsx

@@ -148,7 +148,9 @@ export class ServiceIntegrations extends React.Component<Props, State> {
                panels={[
                  {
                    id: 0,
-                    items: this.getPanelItems(license.features.ml?.is_available)
+                    items: this.getPanelItems(
+                      license.getFeature('ml').isAvailable


Will getFeature always return an object regardless if the plugin is disabled?

Yes. getFeature always returns a LicenseFeature:

kibana/x-pack/plugins/licensing/common/types.ts

Lines 30 to 33 in 88ae8d0

export interface LicenseFeature {

isAvailable: boolean;

isEnabled: boolean;

}

We're using ILicense | undefined now so a ? has been added to license, but the above still holds.

sorenlouv · 2019-12-12T22:49:30Z

x-pack/legacy/plugins/apm/public/context/LicenseContext/index.tsx

 import React from 'react';
-import { FETCH_STATUS, useFetcher } from '../../hooks/useFetcher';
-import { loadLicense, LicenseApiResponse } from '../../services/rest/xpack';


We can also remove the loadLicense method (and the entire rest/xpack.ts file)

That file is deleted.

joshdover

Two small suggestions

joshdover · 2019-12-13T18:18:50Z

x-pack/legacy/plugins/apm/public/components/app/ServiceMap/index.tsx

+  const isValidPlatinumLicense =
+    license !== undefined &&
+    license.status === 'active' &&
+    license.type === 'platinum';


const isValidPlatinumLicense = license?.isActive && license?.type === 'platinum'

Updated in 44d9568.

joshdover · 2019-12-13T18:19:22Z

x-pack/legacy/plugins/apm/public/context/LicenseContext/index.tsx

-    [http]
-  );
-  const hasValidLicense = data.license.is_active;
+  const hasInValidLicense = license !== undefined && !license.isActive;


Suggested change

const hasInValidLicense = license !== undefined && !license.isActive;

const hasInValidLicense = !license?.isActive;

Updated in 44d9568.

sorenlouv · 2019-12-16T08:59:57Z

x-pack/legacy/plugins/apm/public/components/app/ServiceMap/LicensePrompt.tsx

+import { i18n } from '@kbn/i18n';
+import { useKibanaUrl } from '../../../hooks/useKibanaUrl';
+
+export function LicensePrompt() {


Perhaps call it "PlatinumLicensePrompt" Since we already have another prompt that ensures the user has a license at all.

Updated in 44d9568.

sorenlouv · 2019-12-16T09:01:04Z

x-pack/legacy/plugins/apm/public/context/LicenseContext/index.tsx


  // if license is invalid show an error message
-  if (status === FETCH_STATUS.SUCCESS && !hasValidLicense) {
+  if (hasInValidLicense) {


Suggested change

if (hasInValidLicense) {

if (hasInvalidLicense) {

Updated in 44d9568.

sorenlouv · 2019-12-16T09:04:07Z

x-pack/legacy/plugins/apm/public/hooks/useKibanaUrl.ts

+
+export function useKibanaUrl(
+  /** The path to the plugin */ path: string,
+  /** The hash path */ hash: string


The formatting looks odd. Shouldn't the comments either be after the argument:

path: string // The path to the plugin

Or on the line above:

// The path to the plugin path: string

Either way I don't think they are terribly helpful. So perhaps just remove them?

It looks odd (to you) but it's valid JSDoc comments that show up when using autocomplete:

sorenlouv

A few nits

sorenlouv · 2019-12-16T10:48:11Z

@smith can you add a "Closes #issue" to tie the PR and issue together? ~~I think the issue is currently unassigned in the backlog so someone could pick it.~~

lizozom

kibana_react export LGTM

Display a link to platinum license upgrade on the service map. Also add `useKibanaUrl` and `useLicense` hooks. Make the `LicenseContext` (which is used in a couple class components and on page load) use the license observable from the NP plugin. Add missing export of `useObservable` to kibana_react.

legrego · 2019-12-17T16:29:12Z

Removing review request for @elastic/kibana-security since this doesn't touch anything we own. Let me know if you'd still like a review!

smith · 2019-12-17T16:30:37Z

@elasticmachine test this please

smith · 2019-12-17T16:31:16Z

@elasticmachine test this please

elasticmachine · 2019-12-17T17:46:06Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 44d9568

History

💔 Build #16120 failed 44d9568
💚 Build #15651 succeeded 507da23
💚 Build #15493 succeeded 25cb6f7
💚 Build #15487 succeeded 921e593
💚 Build #15458 succeeded b7e5f18

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

Display a link to platinum license upgrade on the service map. Also add `useKibanaUrl` and `useLicense` hooks. Make the `LicenseContext` (which is used in a couple class components and on page load) use the license observable from the NP plugin. Add missing export of `useObservable` to kibana_react.

dgieselaar · 2020-01-16T14:56:13Z

Tested in Chrome, FF, IE11, Safari.

smith force-pushed the nls/map-plat branch from f13aa58 to 92a5b19 Compare December 12, 2019 21:55

smith added release_note:skip Skip the PR/issue when compiling release notes v7.6.0 labels Dec 12, 2019

smith marked this pull request as ready for review December 12, 2019 22:01

smith requested a review from a team as a code owner December 12, 2019 22:01

sorenlouv reviewed Dec 12, 2019

View reviewed changes

mbondyra requested a review from a team December 13, 2019 16:56

mbondyra requested review from a team as code owners December 13, 2019 16:56

smith force-pushed the nls/map-plat branch 2 times, most recently from 921e593 to 25cb6f7 Compare December 13, 2019 18:06

smith requested a review from joshdover December 13, 2019 18:08

joshdover reviewed Dec 13, 2019

View reviewed changes

smith force-pushed the nls/map-plat branch from 25cb6f7 to 507da23 Compare December 15, 2019 18:23

sorenlouv reviewed Dec 16, 2019

View reviewed changes

sorenlouv approved these changes Dec 16, 2019

View reviewed changes

lizozom approved these changes Dec 16, 2019

View reviewed changes

smith force-pushed the nls/map-plat branch from 507da23 to 44d9568 Compare December 17, 2019 15:15

legrego removed the request for review from a team December 17, 2019 16:28

jbudz removed request for a team December 17, 2019 16:46

smith merged commit d391764 into elastic:master Dec 17, 2019

smith deleted the nls/map-plat branch December 17, 2019 18:01

smith mentioned this pull request Jan 10, 2020

[7.x] [APM] License prompt for service map (#52668) #54503

Merged

ogupte added the apm-test-plan-7.6.0 label Jan 15, 2020

dgieselaar self-assigned this Jan 16, 2020

dgieselaar added the apm:test-plan-done Pull request that was successfully tested during the test plan label Jan 16, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[APM] License prompt for service map #52668

[APM] License prompt for service map #52668

smith commented Dec 10, 2019 •

edited

Loading

sorenlouv Dec 12, 2019

smith Dec 13, 2019

sorenlouv Dec 12, 2019

smith Dec 13, 2019

smith Dec 13, 2019

sorenlouv Dec 12, 2019

smith Dec 12, 2019

joshdover left a comment

joshdover Dec 13, 2019

smith Dec 17, 2019

joshdover Dec 13, 2019

smith Dec 17, 2019

sorenlouv Dec 16, 2019

smith Dec 17, 2019

sorenlouv Dec 16, 2019

smith Dec 17, 2019

sorenlouv Dec 16, 2019 •

edited

Loading

smith Dec 17, 2019

sorenlouv left a comment

sorenlouv commented Dec 16, 2019 •

edited

Loading

lizozom left a comment

legrego commented Dec 17, 2019

smith commented Dec 17, 2019 •

edited

Loading

smith commented Dec 17, 2019

elasticmachine commented Dec 17, 2019

dgieselaar commented Jan 16, 2020

	export interface LicenseFeature {
	isAvailable: boolean;
	isEnabled: boolean;
	}

	const hasInValidLicense = license !== undefined && !license.isActive;
	const hasInValidLicense = !license?.isActive;

[APM] License prompt for service map #52668

[APM] License prompt for service map #52668

Conversation

smith commented Dec 10, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshdover left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sorenlouv Dec 16, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sorenlouv left a comment

Choose a reason for hiding this comment

sorenlouv commented Dec 16, 2019 • edited Loading

lizozom left a comment

Choose a reason for hiding this comment

legrego commented Dec 17, 2019

smith commented Dec 17, 2019 • edited Loading

smith commented Dec 17, 2019

elasticmachine commented Dec 17, 2019

💚 Build Succeeded

History

dgieselaar commented Jan 16, 2020

smith commented Dec 10, 2019 •

edited

Loading

sorenlouv Dec 16, 2019 •

edited

Loading

sorenlouv commented Dec 16, 2019 •

edited

Loading

smith commented Dec 17, 2019 •

edited

Loading