[Profiling,Infra,APM] Disable Profiling integration by default #175201

mykolaharmash · 2024-01-22T12:44:04Z

Summary

This PR disables the Profiling integration in Infra and APM by default on the plugin configuration level because this integration require users to first configure the main profiling plugin. On-prem users will have to manually enable both integrations once they enabled the Universal Profiling for their hosts. Cloud users will have Infra and APM integrations enabled by default because on Cloud instances Universal Profiling is already configured. A PR for the default Cloud settings will follow after this one is merged.

Changes I've made:

Disabled the Infra integration be default
Introduced a new APM feature flag for the Profiling integration
Made sure all the places in APM that rely on Profiling integration respect the new feature flag
Fixed a bug in APM when Universal Profiling was shown even though the integration was disabled in UI settings

CleanShot.2024-01-22.at.13.31.40.mp4

How To Test

Checkout locally
Make sure you don't have xpack.infra.featureFlags.profilingEnabled already enabled in kibana.yml
Open kibana and make sure you don't see "Universal Profiling" tab in Host and Service details
Enabled both flags in kibana.yml: xpack.infra.featureFlags.profilingEnabled and xpack.apm.featureFlags.profilingIntegrationAvailable: true
Check that now you see "Universal Profiling" tab in the details screens in both Infra and APM
Go to Infra settings view and disable the Profiling integration, make sure the tab disappears
1. Go to APM settings view and disable the Profiling integration, make sure the tab disappears

apmmachine · 2024-01-22T12:44:20Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
/oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

elasticmachine · 2024-01-22T12:50:22Z

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

mykolaharmash · 2024-01-22T13:37:35Z

/ci

jeramysoucy · 2024-01-22T22:04:04Z

test/plugin_functional/test_suites/core_plugins/rendering.ts

@@ -213,6 +213,7 @@ export default function ({ getService }: PluginFunctionalProviderContext) {
        'xpack.apm.featureFlags.migrationToFleetAvailable (any)',
        'xpack.apm.featureFlags.sourcemapApiAvailable (any)',
        'xpack.apm.featureFlags.storageExplorerAvailable (any)',
+        'xpack.apm.featureFlags.profilingIntegrationAvailable (any)',


Can we specify 'boolean' here due to the strict schema?

kibana/x-pack/plugins/apm/server/index.ts

Line 80 in b8449e7

profilingIntegrationAvailable: schema.boolean({ defaultValue: false }),

Good point, done ✅

…rofiling-integration

jennypavlova

I checked locally with the provided steps and it works as described. I have one question: Should we have the option to enable profiling in the settings if the xpack.profiling.enabled: true and the flags are not set in the kibana.yml? I find it a bit odd to have the option in the settings and for example to disable the profiling there and then to remove the flags from the config and the option to enable it disappears 🤔

profiling_settingd.mov

I think the expected behavior will be to always have the flags in the settings and only the value to change (with false as a default option) Wdyt?

x-pack/plugins/apm/public/index.ts

mykolaharmash · 2024-01-23T11:55:33Z

I checked locally with the provided steps and it works as described. I have one question: Should we have the option to enable profiling in the settings if the xpack.profiling.enabled: true and the flags are not set in the kibana.yml? I find it a bit odd to have the option in the settings and for example to disable the profiling there and then to remove the flags from the config and the option to enable it disappears 🤔

profiling_settingd.mov
I think the expected behavior will be to always have the flags in the settings and only the value to change (with false as a default option) Wdyt?

The things is, if we keep the toggle in Settings while the plugin is disabled, the toggle would have no effect regardless of it's state, because the plugin configuration takes precedence. That's why I decided to hide the toggle altogether if the feature flag is off to not confuse users that might expect something to happen if they toggle it on. Does this makes sense or I'm missing something?

mykolaharmash · 2024-01-23T12:39:10Z

@elasticmachine merge upstream

jennypavlova

The things is, if we keep the toggle in Settings while the plugin is disabled, the toggle would have no effect regardless of it's state, because the plugin configuration takes precedence. That's why I decided to hide the toggle altogether if the feature flag is off to not confuse users that might expect something to happen if they toggle it on.

That makes sense, thanks for explaining!
I found the behavior confusing only in the case when setting it to off in the settings and then if the plugin feature flag is removed from the config and then set to true again it will still be off (because the settings config will be the one used even if it's enabled in the kibana.yml file after it is set to false in the settings)- that's probably an edge case and not likely to happen.

But overall it works fine, thanks for the fixes :)

jeramysoucy

Kibana Security changes LGTM

mykolaharmash · 2024-01-23T15:25:04Z

@elasticmachine merge upstream

…mykolaharmash/kibana into 175016-cloud-only-profiling-integration

…rofiling-integration

mykolaharmash · 2024-01-24T09:49:50Z

That makes sense, thanks for explaining!
I found the behavior confusing only in the case when setting it to off in the settings and then if the plugin feature flag is removed from the config and then set to true again it will still be off (because the settings config will be the one used even if it's enabled in the kibana.yml file after it is set to false in the settings)- that's probably an edge case and not likely to happen.

Ah, now I see what you mean, thank you for clarifying. But I agree that this would be rather an edge-case and I hope if that happens user would remember to check the ui setting when they re-enable the plugin flag because at some point they've toggled it off and at least should be aware that it exists 🤞

kibana-ci · 2024-01-24T11:24:57Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 9af38b5

Failed CI Steps

Test Failures

[job] [logs] FTR Configs #35 / Serverless observability API - feature flags Custom Threshold Rule Custom Threshold rule - DOCUMENTS_COUNT - FIRED Rule creation should set correct information in the alert document

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`apm`	1600	1601	+1

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.1MB	3.1MB	+268.0B

History

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

kibanamachine · 2024-01-24T11:52:14Z

💔 All backports failed

Status	Branch	Result
❌	8.12	Backport failed because of merge conflicts You might need to backport the following PRs to 8.12: - [Infra UI] Fix `Open in Logs` links in Infra and APM when in Serverless (#172137)

Manual backport

To create the backport manually run:

node scripts/backport --pr 175201

Questions ?

Please refer to the Backport tool documentation

kibanamachine · 2024-01-25T21:48:53Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 175201 locally

kibanamachine · 2024-01-27T21:48:57Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 175201 locally

mykolaharmash · 2024-02-07T13:43:49Z

💚 All backports created successfully

Status	Branch	Result
✅	8.12

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…ic#175201) Closes elastic#175016 ## Summary This PR disables the Profiling integration in Infra and APM by default on the plugin configuration level because this integration require users to first configure the main `profiling` plugin. On-prem users will have to manually enable both integrations once they enabled the Universal Profiling for their hosts. Cloud users will have Infra and APM integrations enabled by default because on Cloud instances Universal Profiling is already configured. A PR for the default Cloud settings will follow after this one is merged. Changes I've made: * Disabled the Infra integration be default * Introduced a new APM feature flag for the Profiling integration * Made sure all the places in APM that rely on Profiling integration respect the new feature flag * Fixed a bug in APM when Universal Profiling was shown even though the integration was disabled in UI settings https://github.com/elastic/kibana/assets/793851/65dfbb5b-1850-4d18-a92a-6ad59e0436a3 ## How To Test 1. Checkout locally 2. Make sure you don't have `xpack.infra.featureFlags.profilingEnabled` already enabled in `kibana.yml` 3. Open kibana and make sure you don't see "Universal Profiling" tab in Host and Service details 4. Enabled both flags in `kibana.yml`: `xpack.infra.featureFlags.profilingEnabled` and `xpack.apm.featureFlags.profilingIntegrationAvailable: true` 5. Check that now you see "Universal Profiling" tab in the details screens in both Infra and APM 6. Go to Infra settings view and disable the Profiling integration, make sure the tab disappears 7. 6. Go to APM settings view and disable the Profiling integration, make sure the tab disappears --------- Co-authored-by: Kibana Machine <[email protected]> (cherry picked from commit abd3515) # Conflicts: # x-pack/plugins/apm/public/components/app/settings/general_settings/index.tsx # x-pack/plugins/apm/public/components/shared/transaction_action_menu/transaction_action_menu.test.tsx

…#175201) (#176406) # Backport This will backport the following commits from `main` to `8.12`: - [[Profiling,Infra,APM] Disable Profiling integration by default (#175201)](#175201)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)

…ic#175201) Closes elastic#175016 ## Summary This PR disables the Profiling integration in Infra and APM by default on the plugin configuration level because this integration require users to first configure the main `profiling` plugin. On-prem users will have to manually enable both integrations once they enabled the Universal Profiling for their hosts. Cloud users will have Infra and APM integrations enabled by default because on Cloud instances Universal Profiling is already configured. A PR for the default Cloud settings will follow after this one is merged. Changes I've made: * Disabled the Infra integration be default * Introduced a new APM feature flag for the Profiling integration * Made sure all the places in APM that rely on Profiling integration respect the new feature flag * Fixed a bug in APM when Universal Profiling was shown even though the integration was disabled in UI settings https://github.com/elastic/kibana/assets/793851/65dfbb5b-1850-4d18-a92a-6ad59e0436a3 ## How To Test 1. Checkout locally 2. Make sure you don't have `xpack.infra.featureFlags.profilingEnabled` already enabled in `kibana.yml` 3. Open kibana and make sure you don't see "Universal Profiling" tab in Host and Service details 4. Enabled both flags in `kibana.yml`: `xpack.infra.featureFlags.profilingEnabled` and `xpack.apm.featureFlags.profilingIntegrationAvailable: true` 5. Check that now you see "Universal Profiling" tab in the details screens in both Infra and APM 6. Go to Infra settings view and disable the Profiling integration, make sure the tab disappears 7. 6. Go to APM settings view and disable the Profiling integration, make sure the tab disappears --------- Co-authored-by: Kibana Machine <[email protected]>

mykolaharmash force-pushed the 175016-cloud-only-profiling-integration branch 2 times, most recently from da815e5 to f4df01e Compare January 22, 2024 12:49

mykolaharmash added release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team v8.12.1 labels Jan 22, 2024

mykolaharmash marked this pull request as ready for review January 22, 2024 12:50

mykolaharmash requested review from a team as code owners January 22, 2024 12:50

mykolaharmash mentioned this pull request Jan 22, 2024

Enable APM/Infra+Profiling integrations by default for all Cloud deployments #175202

Closed

[Profiling,Infra,APM] Disable Profiling integration by default

b8449e7

mykolaharmash force-pushed the 175016-cloud-only-profiling-integration branch from f4df01e to b8449e7 Compare January 22, 2024 14:07

jeramysoucy reviewed Jan 22, 2024

View reviewed changes

mykolaharmash added 2 commits January 23, 2024 10:13

Fix tests

ee714ca

Merge remote-tracking branch 'upstream/main' into 175016-cloud-only-p…

a8fa940

…rofiling-integration

mykolaharmash requested a review from jeramysoucy January 23, 2024 09:15

jennypavlova self-requested a review January 23, 2024 09:45

mykolaharmash added 2 commits January 23, 2024 11:20

Add new APM feature flag to the mock definition

3933588

Merge remote-tracking branch 'upstream/main' into 175016-cloud-only-p…

447e366

…rofiling-integration

jennypavlova reviewed Jan 23, 2024

View reviewed changes

x-pack/plugins/apm/public/index.ts Show resolved Hide resolved

Merge branch 'main' into 175016-cloud-only-profiling-integration

08f8477

jennypavlova approved these changes Jan 23, 2024

View reviewed changes

mykolaharmash added 2 commits January 23, 2024 14:23

Add missing settings service mock

a390ff2

Mock profiling feature flag value in tests

c49f7b9

jeramysoucy approved these changes Jan 23, 2024

View reviewed changes

kibanamachine and others added 4 commits January 23, 2024 10:25

Merge branch 'main' into 175016-cloud-only-profiling-integration

dd0b783

fixup! Mock profiling feature flag value in tests

6446e55

Merge branch '175016-cloud-only-profiling-integration' of github.com:…

e0d6792

…mykolaharmash/kibana into 175016-cloud-only-profiling-integration

Merge remote-tracking branch 'upstream/main' into 175016-cloud-only-p…

9af38b5

…rofiling-integration

mykolaharmash merged commit abd3515 into elastic:main Jan 24, 2024
21 checks passed

kibanamachine added the v8.13.0 label Jan 24, 2024

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jan 25, 2024

mykolaharmash mentioned this pull request Feb 7, 2024

[8.12] [Profiling,Infra,APM] Disable Profiling integration by default (#175201) #176406

Merged

kibanamachine added v8.12.2 and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Feb 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Profiling,Infra,APM] Disable Profiling integration by default #175201

[Profiling,Infra,APM] Disable Profiling integration by default #175201

mykolaharmash commented Jan 22, 2024 •

edited by kibanamachine

Loading

apmmachine commented Jan 22, 2024

elasticmachine commented Jan 22, 2024

mykolaharmash commented Jan 22, 2024

jeramysoucy Jan 22, 2024

mykolaharmash Jan 23, 2024

jennypavlova left a comment

mykolaharmash commented Jan 23, 2024

mykolaharmash commented Jan 23, 2024

jennypavlova left a comment

jeramysoucy left a comment

mykolaharmash commented Jan 23, 2024

mykolaharmash commented Jan 24, 2024

kibana-ci commented Jan 24, 2024

kibanamachine commented Jan 24, 2024

kibanamachine commented Jan 25, 2024

kibanamachine commented Jan 27, 2024

mykolaharmash commented Feb 7, 2024

[Profiling,Infra,APM] Disable Profiling integration by default #175201

[Profiling,Infra,APM] Disable Profiling integration by default #175201

Conversation

mykolaharmash commented Jan 22, 2024 • edited by kibanamachine Loading

Summary

How To Test

apmmachine commented Jan 22, 2024

🤖 GitHub comments

elasticmachine commented Jan 22, 2024

mykolaharmash commented Jan 22, 2024

jeramysoucy Jan 22, 2024

Choose a reason for hiding this comment

mykolaharmash Jan 23, 2024

Choose a reason for hiding this comment

jennypavlova left a comment

Choose a reason for hiding this comment

mykolaharmash commented Jan 23, 2024

mykolaharmash commented Jan 23, 2024

jennypavlova left a comment

Choose a reason for hiding this comment

jeramysoucy left a comment

Choose a reason for hiding this comment

mykolaharmash commented Jan 23, 2024

mykolaharmash commented Jan 24, 2024

kibana-ci commented Jan 24, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Async chunks

History

kibanamachine commented Jan 24, 2024

💔 All backports failed

Manual backport

Questions ?

kibanamachine commented Jan 25, 2024

kibanamachine commented Jan 27, 2024

mykolaharmash commented Feb 7, 2024

💚 All backports created successfully

Questions ?

mykolaharmash commented Jan 22, 2024 •

edited by kibanamachine

Loading