[Fleet] Enable per policy outputs #126692

nchaulet · 2022-03-02T15:56:06Z

Summary

Resolve #104985

Allow user to set per policy outputs (that feature is only accessible for user with platinum licence)

UI Changes

With Platinum licence

With an expired platinum licence and previously configured per policy output

Without platinum licence

How to test

you can use this in your kibana.dev.yml to quickly get some outputs to test

xpack.fleet.outputs:
  - id: test1
    name: Elasticsearch 1
    hosts: [https://es1:9200]
    type: "elasticsearch"
    is_default: true
    is_default_monitoring: true
  - id: test2
    name: Elasticsearch 2
    hosts: [https://es2:9200]
    type: "elasticsearch"
    is_default: false
    is_default_monitoring: false
  - id: test3
    name: Elasticsearch 3
    hosts: [https://es3:9200]
    type: "elasticsearch"
    is_default: false
    is_default_monitoring: false

tests

Cover with unit tests:

the policy generation with different output(previously done)
the output option generation client side.
the agent policy output validation server side.

elasticmachine · 2022-03-02T15:56:08Z

Pinging @elastic/fleet (Team:Fleet)

nchaulet · 2022-03-02T15:58:35Z

x-pack/plugins/fleet/common/types/models/agent_policy.ts

@@ -25,8 +25,8 @@ export interface NewAgentPolicy {
  monitoring_enabled?: MonitoringType;
  unenroll_timeout?: number;
  is_preconfigured?: boolean;
-  data_output_id?: string;
-  monitoring_output_id?: string;
+  data_output_id?: string | null;


These fields are nullable to allow to clear them to use the default output

Good note. Would you mind adding that as a comment directly in the code?

nchaulet · 2022-03-02T15:59:49Z

...c/applications/fleet/sections/agent_policy/components/agent_policy_advanced_fields/hooks.tsx

+
+import { useGetOutputs, useLicense } from '../../../../hooks';
+
+export const DEFAULT_OUTPUT_VALUE = '@@##DEFAULT_OUTPUT_VALUE##@@';


the super select component do not support null or '' as a value so I come with that not super elegant solution

nchaulet · 2022-03-02T21:49:01Z

@mostlyjason @dborodyansky In the case we have disabled options (with an expired platinum for example) I am wondering if we can have something similiar to what you designed for the logstash output and policy using APM. (A copy explaining why it's disabled)

joshdover · 2022-03-03T12:49:00Z

x-pack/plugins/fleet/common/types/models/agent_policy.ts

@@ -25,8 +25,8 @@ export interface NewAgentPolicy {
  monitoring_enabled?: MonitoringType;
  unenroll_timeout?: number;
  is_preconfigured?: boolean;
-  data_output_id?: string;
-  monitoring_output_id?: string;
+  data_output_id?: string | null;


Good note. Would you mind adding that as a comment directly in the code?

joshdover · 2022-03-03T12:54:23Z

...c/applications/fleet/sections/agent_policy/components/agent_policy_advanced_fields/hooks.tsx

+  const outputsRequest = useGetOutputs();
+  const licenseService = useLicense();
+
+  const isPlatinium = useMemo(() => licenseService.isPlatinium(), [licenseService]);


I'm curious if our heavy usages of useMemo is really necessary. I find it makes our UI code much harder to read and I don't think it's necessary unless there's an expensive calculation that we need to avoid. In this case, I think everything we're doing here is pretty fast.

I'd be curious to get the entire @elastic/fleet team's input on this.

Yes you are right we are probably using too much useMemo for non heavy computation return scalar values

joshdover · 2022-03-03T12:57:09Z

x-pack/plugins/fleet/server/services/agent_policies/validate_outputs_for_policy.test.ts

+  } as any);
+}
+
+describe('validateOutputForPolicy', () => {


Thank you for these unit tests!

joshdover · 2022-03-03T13:01:41Z

x-pack/plugins/fleet/common/services/license.ts

+    return (
+      this.licenseInformation?.isAvailable &&
+      this.licenseInformation?.isActive &&
+      this.licenseInformation?.hasAtLeast('platinum')
+    );


nit: this could be simpler probably, but then also maybe we don't need these helper methods for each license type?

Suggested change

return (

this.licenseInformation?.isAvailable &&

this.licenseInformation?.isActive &&

this.licenseInformation?.hasAtLeast('platinum')

);

return this.hasAtLeast('platinum');

👍 I used directly the hasAtLeast and refactored the existing helper to use hasAtLeast too

joshdover · 2022-03-03T15:53:29Z

What should the behavior be when an output is removed? When I removed the preconfigured outputs from my kibana.yml, my agent policy that was not using those outputs looked like this and the latest revision doc in .fleet-policies index still pointed to the old outputs.

In the case of removing preconfigured outputs, I wonder if we should actually log a warning when a preconfigured output is removed that is still in use and then don't delete and make it "unmanaged". We definitely shouldn't allow removing the default output from preconfiguration.

For outputs removed from the UI, it seems we already show a warning that this will update X agent policies and affect Y agents (I assume we revert affected policies to the default output) 👍

nchaulet · 2022-03-03T16:07:32Z

What should the behavior be when an output is removed? When I removed the preconfigured outputs from my kibana.yml, my agent policy that was not using those outputs looked like this and the latest revision doc in .fleet-policies index still pointed to the old outputs.

I think something like this could work, I created a new issue, so I can tackle this as a follow up PR and document the decision we take too #126810

kibana-ci · 2022-03-03T16:40:09Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`fleet`	569	570	+1

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
`fleet`	1233	1234	+1

Async chunks

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

id	before	after	diff
`fleet`	661.7KB	664.5KB	+2.8KB

Page load bundle

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

id	before	after	diff
`fleet`	110.5KB	110.3KB	-287.0B

Unknown metric groups

API count

id	before	after	diff
`fleet`	1350	1351	+1

ESLint disabled line counts

id	before	after	diff
`fleet`	41	45	+4

Total ESLint disabled count

id	before	after	diff
`fleet`	49	53	+4

History

💚 Build #27188 succeeded 83fa87b
💔 Build #27147 failed d59fe9f

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

cc @nchaulet

joshdover

Thanks for filing the follow up, LGTM!

kibanamachine · 2022-03-03T19:07:15Z

⚪ Backport skipped

The pull request was not backported as there were no branches to backport to. If this is a mistake, please apply the desired version labels or run the backport tool manually.

Manual backport

To create the backport manually run:

node scripts/backport --pr 126692

Questions ?

Please refer to the Backport tool documentation

dikshachauhan-qasource · 2022-03-24T09:42:14Z

Hi @joshdover

We have validated above UI changes and found available on latest builds of 8.2 snapshot.

Both policy outputs are displayed while creating policy.
Also, when navigated to policy's settings tab.
User is able to select value from dropdown for current policy.
By default, default output is pre selected at both fields.

Screenshot:

Build checked at: BUILD: 51365
COMMIT: 130823a

Further, we have created proposed test cases for same under related ticket (#104987) at doc

We will testing the complete scenario when feature will be fully available and will share our observations.

Thanks
QAS

[Fleet] Enable per policy outputs

268c17e

nchaulet requested a review from a team as a code owner March 2, 2022 15:56

nchaulet self-assigned this Mar 2, 2022

nchaulet commented Mar 2, 2022

View reviewed changes

nchaulet added 3 commits March 2, 2022 11:04

Fix after first review pass

10cb3af

Fix typo

d59fe9f

Fix typo

83fa87b

joshdover reviewed Mar 3, 2022

View reviewed changes

Fix after review

676ec89

nchaulet requested review from joshdover and a team March 3, 2022 15:17

joshdover approved these changes Mar 3, 2022

View reviewed changes

nchaulet merged commit 247d3a5 into elastic:main Mar 3, 2022

nchaulet deleted the feature-enable-per-policy-output branch March 3, 2022 19:07

nchaulet removed the auto-backport Deprecated - use backport:version if exact versions are needed label Mar 3, 2022

ollyhowell mentioned this pull request Apr 14, 2022

[Request] Document per policy outputs for Fleet elastic/observability-docs#1791

Closed

Bearloggs mentioned this pull request Oct 12, 2023

[Fleet] Gold License Denied Access to Set Per Policy Output #168734

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fleet] Enable per policy outputs #126692

[Fleet] Enable per policy outputs #126692

nchaulet commented Mar 2, 2022 •

edited

Loading

elasticmachine commented Mar 2, 2022

nchaulet Mar 2, 2022

joshdover Mar 3, 2022

nchaulet Mar 2, 2022

nchaulet commented Mar 2, 2022

joshdover Mar 3, 2022

joshdover Mar 3, 2022

nchaulet Mar 3, 2022

joshdover Mar 3, 2022

joshdover Mar 3, 2022

nchaulet Mar 3, 2022

joshdover commented Mar 3, 2022

nchaulet commented Mar 3, 2022

kibana-ci commented Mar 3, 2022

API count

ESLint disabled line counts

Total ESLint disabled count

joshdover left a comment

kibanamachine commented Mar 3, 2022

dikshachauhan-qasource commented Mar 24, 2022


		import { useGetOutputs, useLicense } from '../../../../hooks';

		export const DEFAULT_OUTPUT_VALUE = '@@##DEFAULT_OUTPUT_VALUE##@@';

[Fleet] Enable per policy outputs #126692

[Fleet] Enable per policy outputs #126692

Conversation

nchaulet commented Mar 2, 2022 • edited Loading

Summary

UI Changes

With Platinum licence

With an expired platinum licence and previously configured per policy output

Without platinum licence

How to test

tests

elasticmachine commented Mar 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nchaulet commented Mar 2, 2022

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 commented Mar 3, 2022

nchaulet commented Mar 3, 2022

kibana-ci commented Mar 3, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

ESLint disabled line counts

Total ESLint disabled count

History

joshdover left a comment

Choose a reason for hiding this comment

kibanamachine commented Mar 3, 2022

⚪ Backport skipped

Manual backport

Questions ?

dikshachauhan-qasource commented Mar 24, 2022

nchaulet commented Mar 2, 2022 •

edited

Loading