[Telemetry] Make `telemetry` plugin non-disableable #133205

afharo · 2022-05-31T11:33:16Z

Summary

Some conversations around #129015 led to highlight the reasons for deprecating telemetry.enabled: false are purely technical and they shouldn't affect how the feature works from the users' POV.

This PR internally converts telemetry.enabled: false to telemetry.optIn: false + telemetry.allowChangingOptInStatus: false without actually disabling the telemetry plugin. Both flags should provide feature parity without disabling the plugin.

Why do we not want to disable the plugin?

The reasoning is already explained in #129015, but the main reasons are:

Saved Objects migrations won't fail in deployments where telemetry was previously enabled.
We don't need a workaround to call optIn(false) in EBT.

Checklist

Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Users might be concerned that they can see in the logs that the plugin is still enabled and the bundles loaded in the UI when they set `telemetry.enabled: false`	Medium	Low	Documentation making clear that `telemetry.optIn: false` doesn't send any data to Elastic should address those concerns.
External consumers hitting the HTTP API `POST /api/telemetry/v2/clusters/_stats` may generate a high load	Low	High	It's an undocumented API, and should not be used. If a user is polling that API, they can simply configure their external systems to stop polling it.

For maintainers

This was checked for breaking API changes and was labeled appropriately

afharo · 2022-06-01T08:06:41Z

docs/settings/telemetry-settings.asciidoc

-[[telemetry-enabled]] `telemetry.enabled`::
-  Set to `true` to send cluster statistics to Elastic. Reporting your
-  cluster statistics helps us improve your user experience. Your data is never
-  shared with anyone. Set to `false` to disable statistics reporting from any
-  browser connected to the {kib} instance. Defaults to `true`.


Removing documentation for telemetry.enabled so users are not tempted to use it. Even when we are not "officially" deprecating them and it will work if they set it.

Is this OK?

afharo · 2022-06-01T08:07:05Z

src/plugins/kibana_usage_collection/kibana.json

-  "optionalPlugins": [
-    "telemetry"
-  ]


The workaround is no-longer needed 😇

afharo · 2022-06-01T08:07:55Z

src/plugins/kibana_usage_collection/public/plugin.test.ts

 import { plugin } from '.';

 describe('kibana_usage_collection/public', () => {
  const pluginInstance = plugin();

-  describe('optIn fallback from telemetry', () => {
-    test('should call optIn(false) when telemetry is disabled', () => {
+  describe('EBT stats -> UI Counters', () => {


I didn't want to leave this test file empty, so I repurposed the tests to validate another piece of logic in the plugin.

afharo · 2022-06-01T08:10:06Z

src/plugins/telemetry/server/config/config.ts

+    (cfg) => {
+      if (cfg.telemetry?.enabled === false) {
+        return {
+          set: [
+            { path: 'telemetry.optIn', value: false },
+            { path: 'telemetry.allowChangingOptInStatus', value: false },
+          ],
+          unset: [{ path: 'telemetry.enabled' }],
+        };
+      }
+    },


Using a custom config deprecation provider to be able to set 2 values out of 1 and to avoid logging any deprecation warning (hence not showing the warning in the Upgrade Assistant).

elasticmachine · 2022-06-24T17:43:29Z

Pinging @elastic/kibana-core (Team:Core)

azasypkin

Changes in expectedExposedConfigKeys LGTM.

afharo · 2022-06-27T09:01:21Z

@elasticmachine merge upstream

kibana-ci · 2022-06-27T10:05:21Z

💚 Build Succeeded

Metrics [docs]

Page load bundle

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

id	before	after	diff
`kibanaUsageCollection`	1.5KB	1.4KB	-55.0B

Unknown metric groups

API count

id	before	after	diff
`telemetry`	44	43	-1

History

💚 Build #53201 succeeded bd8bc03
💚 Build #48692 succeeded 532f94a762436c5e48839e5097884493ce129cbb
💔 Build #48642 failed 7f8c00068c93230be40403e986d8394fcb5b87ff
💔 Build #48602 failed cf7c33b4463bec2fc818b2d46cc0a18bc2687baf
💚 Build #48556 succeeded eb14757e35a5377a833f2e3f6ab15cc8e2787f5c
💔 Build #48435 failed 325cfad0138338ed569c535f74cb32be0300adb7

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

pgayvallet

Got a question, apart that LGTM

pgayvallet · 2022-06-27T14:07:44Z

src/plugins/kibana_usage_collection/public/plugin.ts

-    if (!telemetry) {
-      // If the telemetry plugin is disabled, let's set optIn false to flush the queues.
-      coreSetup.analytics.optIn({ global: { enabled: false } });
-    }


Shouldn't we base this on telemetry.optIn now?

This was a fallback to overcome that telemetry was disabled and wouldn't call analytics.optIn(false) so that EBT would stop holding the events until an optIn response was provided.

Since the telemetry plugin cannot be disabled now, it will always call analytics.optIn(true|false) so we don't need this workaround anymore.

pgayvallet · 2022-06-27T14:08:04Z

src/plugins/kibana_usage_collection/server/plugin.ts

-    if (!telemetry) {
-      // If the telemetry plugin is disabled, let's set optIn false to flush the queues.
-      coreSetup.analytics.optIn({ global: { enabled: false } });
-    }


Same question here?

afharo added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:Telemetry release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting v8.4.0 labels May 31, 2022

afharo force-pushed the non-disableable-telemetry-plugin branch 6 times, most recently from 7f8c000 to 532f94a Compare June 1, 2022 15:28

[Telemetry] Make telemetry plugin non-disableable

bd8bc03

afharo force-pushed the non-disableable-telemetry-plugin branch from 532f94a to bd8bc03 Compare June 24, 2022 16:26

afharo commented Jun 24, 2022

View reviewed changes

afharo marked this pull request as ready for review June 24, 2022 17:43

afharo requested review from a team as code owners June 24, 2022 17:43

azasypkin approved these changes Jun 27, 2022

View reviewed changes

Merge branch 'main' into non-disableable-telemetry-plugin

767102c

pgayvallet approved these changes Jun 27, 2022

View reviewed changes

afharo merged commit 2410b87 into elastic:main Jun 27, 2022

afharo deleted the non-disableable-telemetry-plugin branch June 27, 2022 14:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Telemetry] Make `telemetry` plugin non-disableable #133205

[Telemetry] Make `telemetry` plugin non-disableable #133205

afharo commented May 31, 2022 •

edited

Loading

afharo Jun 1, 2022

afharo Jun 1, 2022

afharo Jun 1, 2022

afharo Jun 1, 2022

elasticmachine commented Jun 24, 2022

azasypkin left a comment

afharo commented Jun 27, 2022

kibana-ci commented Jun 27, 2022

API count

pgayvallet left a comment

pgayvallet Jun 27, 2022

afharo Jun 27, 2022

pgayvallet Jun 27, 2022

[Telemetry] Make telemetry plugin non-disableable #133205

[Telemetry] Make telemetry plugin non-disableable #133205

Conversation

afharo commented May 31, 2022 • edited Loading

Summary

Why do we not want to disable the plugin?

Checklist

Risk Matrix

For maintainers

afharo Jun 1, 2022

Choose a reason for hiding this comment

afharo Jun 1, 2022

Choose a reason for hiding this comment

afharo Jun 1, 2022

Choose a reason for hiding this comment

afharo Jun 1, 2022

Choose a reason for hiding this comment

elasticmachine commented Jun 24, 2022

azasypkin left a comment

Choose a reason for hiding this comment

afharo commented Jun 27, 2022

kibana-ci commented Jun 27, 2022

💚 Build Succeeded

Metrics [docs]

Page load bundle

API count

History

pgayvallet left a comment

Choose a reason for hiding this comment

pgayvallet Jun 27, 2022

Choose a reason for hiding this comment

afharo Jun 27, 2022

Choose a reason for hiding this comment

pgayvallet Jun 27, 2022

Choose a reason for hiding this comment

[Telemetry] Make `telemetry` plugin non-disableable #133205

[Telemetry] Make `telemetry` plugin non-disableable #133205

afharo commented May 31, 2022 •

edited

Loading