[APM] Agent remote configuration #39555

ogupte · 2019-06-25T05:21:44Z

Summary

Closes #34990. Adds support for remote configuration of agents within the APM UI.

This is based off the work of this PR: #36031.

x-pack/legacy/plugins/apm/server/lib/settings/cm/save_configuration.ts

elasticmachine · 2019-06-25T06:33:22Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

simitt · 2019-06-25T15:43:53Z

@ogupte I tested APM Server playing together with this branch. I did find some unexpected behavior related to service.environment. Entries in ES that only contain a service.name but no service.environment should not be returned if the query only contains service.name. Same thing the other way around, if the query contains service.name and service.environment no results that are not matching the service.environment part should be returned. During my tests entries that should not match based on the environment were returned in the result. Do you have any tests for that?

ogupte · 2019-06-25T16:02:04Z

@ogupte I tested APM Server playing together with this branch. I did find some unexpected behavior related to service.environment. Entries in ES that only contain a service.name but no service.environment should not be returned if the query only contains service.name. Same thing the other way around, if the query contains service.name and service.environment no results that are not matching the service.environment part should be returned. During my tests entries that should not match based on the environment were returned in the result. Do you have any tests for that?

I'll make sure to address this and add tests to make sure only entries with both service.name and service.environment are returned. Thank you for checking this.

elasticmachine · 2019-06-26T09:07:34Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-06-26T21:21:55Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-06-27T09:08:18Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

formgeist

@ogupte Starting to take shape and it's looking really good. I have a few nits and changes to request.

x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyout.tsx

x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyoutBody.tsx

formgeist · 2019-06-27T11:06:37Z

x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyoutBody.tsx

@@ -143,7 +169,7 @@ export function AddSettingFlyoutBody({ onSubmit }: { onSubmit: () => void }) {
          <EuiFieldNumber
            min={0}
            max={1}
-            step={0.1}
+            step={0.001}


I think this might need to change to 0.01 as I don't think we need to support 0.5%. @graphaelli thoughts?

Notes from Slack conversation: we agreed to keep the 0.001 validation because it should be possible for users to do 0.1% sample rate configuration.

formgeist · 2019-06-27T11:10:42Z

I forgot to add this in as a note yesterday after the APM weekly, but there's a couple of larger things to address;

"All environments" option

Perhaps because the Opbeans apps don't have multiple environments, I haven't been able to test out the functionality in which a user is able to select "all environments" option which will bulk create configurations for each environment for the selected service with the set sample rate. Remind me, is this possible in this current implementation?

Minor design feedbacks

The services list is currently not sorted alphabetically.

The Flyout should not pre-fill the name and environment selects upon loading. In some cases, the user will have created a configuration for the preselected values and that's where they see an error.
The sample rate input field could use some stricter input validation. I understand that 00000000 results in 0, but perhaps having it restricted to max. 4 chars (because x.xx is the max amount of decimals we allow).

ogupte · 2019-06-27T19:07:20Z

@formgeist

"All environments" option

Perhaps because the Opbeans apps don't have multiple environments, I haven't been able to test out the functionality in which a user is able to select "all environments" option which will bulk create configurations for each environment for the selected service with the set sample rate. Remind me, is this possible in this current implementation?

We could do something like this, but it would be a shortcut to adding one configuration per each existing environment. This means if a new environment comes online after this configuration is set, it would not apply to the new environment. I'm not sure if users will understand this subtly. Also, would selecting 'all environments' like this apply to those services with no value in service.environemnt?

formgeist · 2019-06-27T19:17:46Z

@ogupte I can see how it might be confusing to users that 1. is it going to work for my future environments and 2. how does it apply to services where environment is not set. I reckon we leave it for now. It's more of a shortcut or making that bulk action easier for the user. Let's focus on getting this and other planned features in good shape for next week 👍

x-pack/legacy/plugins/apm/public/components/app/Settings/ListSettings.tsx

x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyout.tsx

elasticmachine · 2019-06-28T06:12:38Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

formgeist · 2019-06-28T08:52:30Z

@bmorelli25 I'm not sure if you already did a pass on the copy, but I would like some help on possible improvements. Thanks!

elasticmachine · 2019-06-28T09:57:27Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

bmorelli25

@formgeist - looks like you updated the copy since yesterday. I'm liking the changes! Just a few recommendations.

x-pack/legacy/plugins/apm/public/components/app/Settings/ListSettings.tsx

x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyoutBody.tsx

x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyout.tsx

x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyoutBody.tsx

elasticmachine · 2019-06-28T19:44:25Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-06-29T01:52:00Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-06-29T05:43:30Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

- Creates apmCm index mapping on plugin initialization. - Links from home page to settings page - Polishes settings page to match design

- Adds ability to update/edit existing setting value - Adds delete functionality to edit settings flyout

…ttings/AddSettingFlyout.tsx sentence updated to remove "central" Co-Authored-By: Brandon Morelli <bmorelli25@gmail.com>

…ettings.tsx fix wording Co-Authored-By: Brandon Morelli <bmorelli25@gmail.com>

…ttings/AddSettingFlyoutBody.tsx fix typo Co-Authored-By: Brandon Morelli <bmorelli25@gmail.com>

…ttings/AddSettingFlyoutBody.tsx fix wording Co-Authored-By: Brandon Morelli <bmorelli25@gmail.com>

Co-Authored-By: Brandon Morelli <bmorelli25@gmail.com>

- adds last updated to config table - fixes validation for all form fields - adds text for toast notifications (create,edit,delete) - makes service name clickable to open edit flyout - adds i18n for all text

- memoized returned object from useFetcher when data doesn't change - changed delete query refresh from true to 'wait_for' - added ignore_above: 1024 to all keyword typed fields in .apm-cm index - fixed some wording around sample_rate granularity - various code cleanup

elasticmachine · 2019-07-02T19:11:09Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

resolved

* [APM] Agent remote configuration UI - Creates apmCm index mapping on plugin initialization. - Links from home page to settings page

* [APM] Agent remote configuration (#39555) * [APM] Agent remote configuration UI - Creates apmCm index mapping on plugin initialization. - Links from home page to settings page * [APM] removes apm agent config index from list of indices that match the apm index patter. Fixes smoke tests.

sorenlouv · 2019-07-03T13:17:11Z

x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyout.tsx

+    selectedConfig ? parseFloat(selectedConfig.settings.sample_rate) : NaN
+  );
+  const { data: serviceNames = [], status: serviceNamesStatus } = useFetcher<
+    string[]


You don't need to supply a type to useFetcher. It is inferred from the promise return type.

sorenlouv · 2019-07-03T13:18:47Z

x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyout.tsx

+    string[]
+  >(async () => (await loadCMServices()).sort(), [], {
+    preservePreviousResponse: false
+  });


I think you should move the sort to the rest method (loadCMServices) and jsut do:

useFetcher(loadCMServices, [], { preservePreviousResponse: false });

sorenlouv · 2019-07-03T13:19:49Z

x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyout.tsx

+  });
+  const { data: environments = [], status: environmentStatus } = useFetcher<
+    Array<{ name: string; available: boolean }>
+  >(


Same as above: no type needs to be supplied to useFetcher

ogupte added the release_note:enhancement label Jun 25, 2019

ogupte self-assigned this Jun 25, 2019

ogupte mentioned this pull request Jun 25, 2019

[APM] Central configuration management #36031

Closed

2 tasks

ogupte commented Jun 25, 2019

View reviewed changes

x-pack/legacy/plugins/apm/server/lib/settings/cm/save_configuration.ts Outdated Show resolved Hide resolved

ogupte added release_note:roadmap and removed release_note:enhancement labels Jun 26, 2019

formgeist requested changes Jun 27, 2019

View reviewed changes

formgeist mentioned this pull request Jun 27, 2019

[APM] Design - Central config management for agents #29864

Closed

formgeist reviewed Jun 27, 2019

View reviewed changes

x-pack/legacy/plugins/apm/public/components/app/Settings/ListSettings.tsx Outdated Show resolved Hide resolved

bmorelli25 reviewed Jun 27, 2019

View reviewed changes

x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyout.tsx Outdated Show resolved Hide resolved

bmorelli25 reviewed Jun 28, 2019

View reviewed changes

x-pack/legacy/plugins/apm/public/components/app/Settings/AddSettings/AddSettingFlyoutBody.tsx Outdated Show resolved Hide resolved

ogupte marked this pull request as ready for review June 29, 2019 00:53

ogupte requested a review from a team as a code owner June 29, 2019 00:53

ogupte force-pushed the central-config-management branch from 0534851 to c35a8ee Compare June 29, 2019 04:42

sorenlouv and others added 17 commits July 2, 2019 08:21

[APM] Central configuration management

fdef90c

[APM] Agent remote configuration UI

3ebb48a

- Creates apmCm index mapping on plugin initialization. - Links from home page to settings page - Polishes settings page to match design

Copy edits on ListSettings page

4696661

Updated snapshot

da8c4e9

- Updates setting config flyout to match design.

3cc5a66

- Adds ability to update/edit existing setting value - Adds delete functionality to edit settings flyout

Updated naming

e3ceab3

Updated help texts in Flyout

54e2c84

Update x-pack/legacy/plugins/apm/public/components/app/Settings/AddSe…

d64d580

…ttings/AddSettingFlyout.tsx sentence updated to remove "central" Co-Authored-By: Brandon Morelli <bmorelli25@gmail.com>

Updated tooltip and callout content

9c7b380

Converted beta label and desc to callout

1e4ab1b

Update x-pack/legacy/plugins/apm/public/components/app/Settings/ListS…

6190fbe

…ettings.tsx fix wording Co-Authored-By: Brandon Morelli <bmorelli25@gmail.com>

Update x-pack/legacy/plugins/apm/public/components/app/Settings/AddSe…

462eb54

…ttings/AddSettingFlyoutBody.tsx fix typo Co-Authored-By: Brandon Morelli <bmorelli25@gmail.com>

Update x-pack/legacy/plugins/apm/public/components/app/Settings/AddSe…

eca32b6

…ttings/AddSettingFlyoutBody.tsx fix wording Co-Authored-By: Brandon Morelli <bmorelli25@gmail.com>

Deduplicate copy between callout and empty state message

a775878

Co-Authored-By: Brandon Morelli <bmorelli25@gmail.com>

- moves save config logic up one level so it can use button in footer

bf95937

- adds last updated to config table - fixes validation for all form fields - adds text for toast notifications (create,edit,delete) - makes service name clickable to open edit flyout - adds i18n for all text

- PR feedback and fixed errors from CI

f1a98b0

ogupte force-pushed the central-config-management branch from a7f71ad to f734012 Compare July 2, 2019 18:12

ogupte merged commit 0e8b722 into elastic:master Jul 2, 2019

ogupte mentioned this pull request Jul 2, 2019

[7.x] [APM] Agent remote configuration (#39555) #40204

Merged

ogupte added a commit to ogupte/kibana that referenced this pull request Jul 2, 2019

[APM] Agent remote configuration (elastic#39555)

5f3af29

* [APM] Agent remote configuration UI - Creates apmCm index mapping on plugin initialization. - Links from home page to settings page

sorenlouv reviewed Jul 3, 2019

View reviewed changes

bmorelli25 added release_note:enhancement and removed release_note:roadmap labels Jul 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[APM] Agent remote configuration #39555

[APM] Agent remote configuration #39555

ogupte commented Jun 25, 2019 •

edited

Loading

elasticmachine commented Jun 25, 2019

simitt commented Jun 25, 2019

ogupte commented Jun 25, 2019

elasticmachine commented Jun 26, 2019

elasticmachine commented Jun 26, 2019

elasticmachine commented Jun 27, 2019

formgeist left a comment

formgeist Jun 27, 2019

formgeist Jul 1, 2019

formgeist commented Jun 27, 2019 •

edited

Loading

ogupte commented Jun 27, 2019 •

edited

Loading

formgeist commented Jun 27, 2019

elasticmachine commented Jun 28, 2019

formgeist commented Jun 28, 2019

elasticmachine commented Jun 28, 2019

bmorelli25 left a comment

elasticmachine commented Jun 28, 2019

elasticmachine commented Jun 29, 2019

elasticmachine commented Jun 29, 2019

elasticmachine commented Jul 2, 2019

sorenlouv Jul 3, 2019

sorenlouv Jul 3, 2019

sorenlouv Jul 3, 2019

[APM] Agent remote configuration #39555

[APM] Agent remote configuration #39555

Conversation

ogupte commented Jun 25, 2019 • edited Loading

Summary

elasticmachine commented Jun 25, 2019

💚 Build Succeeded

simitt commented Jun 25, 2019

ogupte commented Jun 25, 2019

elasticmachine commented Jun 26, 2019

💔 Build Failed

elasticmachine commented Jun 26, 2019

💔 Build Failed

elasticmachine commented Jun 27, 2019

💚 Build Succeeded

formgeist left a comment

Choose a reason for hiding this comment

formgeist Jun 27, 2019

Choose a reason for hiding this comment

formgeist Jul 1, 2019

Choose a reason for hiding this comment

formgeist commented Jun 27, 2019 • edited Loading

ogupte commented Jun 27, 2019 • edited Loading

formgeist commented Jun 27, 2019

elasticmachine commented Jun 28, 2019

💚 Build Succeeded

formgeist commented Jun 28, 2019

elasticmachine commented Jun 28, 2019

💔 Build Failed

bmorelli25 left a comment

Choose a reason for hiding this comment

elasticmachine commented Jun 28, 2019

💔 Build Failed

elasticmachine commented Jun 29, 2019

💔 Build Failed

elasticmachine commented Jun 29, 2019

💔 Build Failed

elasticmachine commented Jul 2, 2019

💚 Build Succeeded

sorenlouv Jul 3, 2019

Choose a reason for hiding this comment

sorenlouv Jul 3, 2019

Choose a reason for hiding this comment

sorenlouv Jul 3, 2019

Choose a reason for hiding this comment

ogupte commented Jun 25, 2019 •

edited

Loading

formgeist commented Jun 27, 2019 •

edited

Loading

ogupte commented Jun 27, 2019 •

edited

Loading