Self-managed telemetry Privacy Statement notice shown in Cloud environment

[alexfrancoeur]

Kibana version:
7.15 BC3 on Cloud

Describe the bug:
The telemetry notice should not be shown in Cloud as it's covered in the EULA

We've already removed the To stop collection, disable usage data here link on the page portion of the notice

The remaining task is to remove the To learn more about how usage data helps us manage and improve our products and services, see our Privacy Statement link

Steps to reproduce:

1. Spin up Kibana instance on Elastic Cloud
2. Create new space
3. Navigate to space for the first time
4. Telemetry notice shown

Expected behavior:
Not telemetry notice or CTA shown on the interstitial page

Screenshots (from before we removed the last part of the text in #116867):

Any additional context:
Unless we're doing something different in the UI now, there should be settings in the kibana.yml file pre-configured in Cloud to disable this: https://www.elastic.co/guide/en/kibana/current/telemetry-settings-kbn.html

[elasticmachine]

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

[pgayvallet]

This is from here:

kibana/src/plugins/home/public/application/components/welcome.tsx

Lines 146 to 152 in 66a06f9

	{!!telemetry && (
	<Fragment>
	<EuiTextColor className="euiText--small" color="subdued">
	<FormattedMessage
	id="home.dataManagementDisclaimerPrivacy"
	defaultMessage="To learn about how usage data helps us manage and improve our products and services, see our "
	/>

ATM, the only condition to display this notice is the presence of the telemetry plugin (if the plugin is enabled, we display the message).

@alexfrancoeur should the rule be changed to only display it if telemetry is enabled and if we're not on cloud?

By greping the message, I see another occurrence, that seems to be displayed as a notification

kibana/src/plugins/telemetry/public/services/telemetry_notifications/telemetry_notifications.ts

Lines 48 to 53 in ed2e544

	public renderOptedInNoticeBanner = (): void => {
	const bannerId = renderOptedInNoticeBanner({
	http: this.http,
	onSeen: this.setOptedInNoticeSeen,
	overlays: this.overlays,
	});

If we hide the message from the home, we may want to also adapt the logic in shouldShowOptedInNoticeBanner?

kibana/src/plugins/telemetry/public/plugin.ts

Lines 225 to 233 in b6a9f03

	private maybeShowOptedInNotificationBanner() {
	if (!this.telemetryNotifications) {
	return;
	}
	const shouldShowBanner = this.telemetryNotifications.shouldShowOptedInNoticeBanner();
	if (shouldShowBanner) {
	this.telemetryNotifications.renderOptedInNoticeBanner();
	}
	}

Note that checking if we're on cloud may be tricky, as the cloud plugin already depends on home (so we can't easily have home depends on the cloud plugin).

This 'are we on cloud env' need comes back frequently. Maybe we should have this information exposed from core (and have the cloud plugin call an API to notify that we're on cloud)?

cc @afharo

btw, @alexfrancoeur, do you know if the isCloudEnabled API from the cloud plugin is what we should be using here?

[afharo]

Thanks, @alexfrancoeur for opening this issue. I thought we had one somewhere, but I couldn't find it.

[lukeelmers]

Just took a look at this as well. I was able to reproduce on a fresh 7.14.1 cloud deployment, so I do not think it is a regression, however something is definitely broken in our logic (and has been from the beginning)

ATM, the only condition to display this notice is the presence of the telemetry plugin (if the plugin is enabled, we display the message).

This is true for the "to learn about usage data" section, but for the "to stop collection, disable usage data here" line, we are also checking userCanChangeSettings:

kibana/src/plugins/home/public/application/components/welcome.tsx

Lines 78 to 82 in 61503fb

	private renderTelemetryEnabledOrDisabledText = () => {
	const { telemetry } = this.props;
	if (!telemetry || !telemetry.telemetryService.userCanChangeSettings) {
	return null;
	}

userCanChangeSettings will be true if someone has SOM edit permissions (which, of course, Cloud admins will have by default):

kibana/src/plugins/telemetry/public/plugin.ts

Lines 208 to 210 in b6a9f03

	private getCanUserChangeSettings(application: ApplicationStart): boolean {
	return (application.capabilities?.savedObjectsManagement?.edit as boolean | undefined) ?? true;
	}

I think if we added a check for allowChangingOptInStatus, it would result in the expected behavior, because then the only way to see the message would be if:

1. telemetry plugin is enabled
2. user has feature controls permissions to edit stuff in saved objects management
3. telemetry.allowChangingOptInStatus is true

1 & 2 will be true for Cloud users spinning up a fresh deployment, but 3 should always be set to false in Elastic Cloud, so I think it would likely resolve this issue.

My remaining questions for @alexfrancoeur and @thesmallestduck are:

1. Should we change the logic elsewhere as Pierre suggested?
2. Is it okay to still show the "learn about usage data" section when telemetry is active, even if we hide the link to update your opt in/out preferences? (Or perhaps this is even necessary for legal reasons?) Hopefully yes -- if we wanted to hide that on Cloud specifically, it would introduce a big circular dependency issue as Pierre mentioned.

[TinaHeiligers]

I think if we added a check for allowChangingOptInStatus, it would result in the expected behavior,

The plan sounds reasonable to me. Aside: I wish we could clean this logic up a bit but I can't think of a cleaner way to implement it right now.

[lukeelmers]

Should we change the logic elsewhere as Pierre suggested?

Is it okay to still show the "learn about usage data" section when telemetry is active, even if we hide the link to update your opt in/out preferences? (Or perhaps this is even necessary for legal reasons?)

chatted with @thesmallestduck, and we decided the answer to both of these is "yes"

[alexfrancoeur]

Just wanted to ping this again as I noticed this bug in a 7.15 production trial deployment. Can we prioritize this fix in a patch release by any chance?

Responding to the comments above, I'm not sure if we need to show the "learn more" link. This is presented to them as part of the EULA they agree to when signing up for Cloud. It seems like duplicative information and from my previous experience, it is not legally necessary. Though we can follow up with legal on that one.

[lukeelmers]

Can we prioritize this fix in a patch release by any chance?

@alexfrancoeur Yep, I think we can likely get this in a 7.15.x patch release, just not sure which one. We are still focused on critical 7.16 priorities, but we have this on our list of items to pick up when we have the capacity.

I'm not sure if we need to show the "learn more" link. This is presented to them as part of the EULA they agree to when signing up for Cloud. It seems like duplicative information

This part is a little tricky. As Pierre mentioned in his comment above, this creates a circular dependency: the cloud plugin already depends on home, so we can't let home depend on the cloud plugin, which is what it would need to do in order to sort out whether this is a cloud deployment and then conditionally show the learn more link.

There are, of course, ways around this, but they involve restructuring our plugins to get rid of the circular dependency, and changes of that nature are likely not something we'd want to backport to a patch release.

My recommendation, if you think it'd be an acceptable UX, would be to simply hide the link to opt in/out as a first step, and treat the learn more link as a follow-up which likely wouldn't be backported as far.

[pgayvallet]

so we can't let home depend on the cloud plugin, which is what it would need to do in order to sort out whether this is a cloud deployment

This will definitely not be ready for any 7.15 patch or even 7.16, but for more mid-term considerations: Should I open an issue to discuss about exposing some cloud related API (thinking about isCloudEnabled mostly) from core?

[lukeelmers]

This will definitely not be ready for any 7.15 patch or even 7.16

++ To be clear, I was suggesting getting a fix in 7.15.x where we remove the opt in/out link. The learn more link is a bigger question.

Should I open an issue to discuss about exposing some cloud related API

Yeah IMHO we should... this just keeps coming up so it might be time to make a decision on it.

[pgayvallet]

Should I open an issue to discuss about exposing some cloud related API

Opened #113734

[alexfrancoeur]

Thanks for following up folks. When this functionality was originally introduced in 7.5, we did not showcase the message on Cloud. At some point something must have changed. We have a number of different configurations that modified what was seen in the UI (https://www.elastic.co/guide/en/kibana/7.15/telemetry-settings-kbn.html).

My recommendation, if you think it'd be an acceptable UX, would be to simply hide the link to opt in/out as a first step, and treat the learn more link as a follow-up which likely wouldn't be backported as far.

++ I'd be fine with that. Would this also impact the telemetry banner should the interstitial welcome page not show? I'm not sure if this is also now being incorrectly shown on Cloud.

[alexfrancoeur]

Following up on this because I saw that the notice is on Cloud staging with a 7.16 snapshot. Are we able to resolve this issue and backport before the 7.16 release?

[pgayvallet]

Are we able to resolve this issue and backport before the 7.16 release?

In #113734, we came to the consensus that we did not want to have core aware of the cloud configuration, and found an alternative to address the need to silent some specific deprecations when we're running on cloud.

Now, for this specific telemetry issue, as atm cloud depends on home, we can't have a reverse dependency from home to cloud. Fixing this issue would need to split cloud into two distinct plugins, one exposing only the APIs/config access (cloud - need to preserve the plugin id to avoid changing all the consumer plugins' code), and one effectively performing the registrations to home (cloud-registrations?)

That way, we'll have:

• home -> cloud
• cloud-registration -> home + cloud

@lukeelmers as this is a bug, this could be done post FF. However the changes, even if not enormous, are important, are we confident trying to do this for 7.16.0?

[lukeelmers]

@pgayvallet I think we should try to remove the To stop collection, disable usage data here link in 7.16, as this won't require us to deal with the circular dependency issues and therefore makes it a smaller change to merge post 7.16 FF. If we did this in the next week or so, we should also be able to get it in to 7.15.2. I should have some time this week and can tackle this part since there's nothing stopping us from moving forward on it.

Then we could treat the (IMHO less critical) learn more link removal as a follow-up task, which buys us a little more time to untangle the circular dependencies, while knowing we've addressed the most important part of this bug.

@alexfrancoeur Are you comfortable with that plan, or is the learn more link of equal concern to you? I think it would be safer to do a fix for learn more separately -- once we know with certainty what our plan is, it will be easier to assess the risk of backporting.

[alexfrancoeur]

@lukeelmers @pgayvallet this sounds like a good plan to me!

Then we could treat the (IMHO less critical) learn more link removal as a follow-up task, which buys us a little more time to untangle the circular dependencies, while knowing we've addressed the most important part of this bug.

I agree it's less critical, but I'd like to continue to track this piece of work.

[lukeelmers]

I agree it's less critical, but I'd like to continue to track this piece of work

👍 Sounds good. I have a PR up to address the first part: #116867

We can leave this issue open until we are able to address the rest of the issue.

[afharo]

We can leave this issue open until we are able to address the rest of the issue.

@lukeelmers do you think it's worth changing the title and description so we know the remaining scope of the issue?

I think we are referring to:

Note that checking if we're on cloud may be tricky, as the cloud plugin already depends on home (so we can't easily have home depends on the cloud plugin).

This 'are we on cloud env' need comes back frequently. Maybe we should have this information exposed from core (and have the cloud plugin call an API to notify that we're on cloud)?

[lukeelmers]

@afharo We've already removed the To stop collection, disable usage data here link on the page. The remaining task here from a UX perspective is to also remove the To learn more about how usage data helps us manage and improve our products and services, see our Privacy Statement link.

In order to do that, we need to align on the items you mention above (basically, designing a way to do this so we don't create a cloud -> home -> cloud circular dependency).

I'll update the issue description to clarify.

[afharo]

FYI: #126238 moves the piece of logic to the telemetry plugin. AFAIK, it will solve the circular dependency, and we'll be able to check on cloud's API from the telemetry plugin itself.

Once it's merged, we'll be able to add the additional piece of logic to fully hide the privacy statement link on Cloud deployments.

[gsoldevila]

As @afharo mentioned, the circular dependency is no longer an issue thanks to code refactoring.

Now, ATM there are 4 places where the Privacy statement notice can appear:

1. The welcome page interstitial.
   

   • Here, we want to hide the privacy statement [link | paragraph?] for cloud users.

2. The notice banner under the header bars.
   Current display conditions: pluginConfig.telemetryNotifyUserAboutOptInDefault && pluginConfig.userCanChangeSettings
   

   • This notice does NOT appear on a fresh cloud instance (v.8.1.0) with the default settings. ATM it can't be enabled, as cloud forces the allowChangingOptInStatus=false and the setting is not part of the allowlist.

3. The popover that appears on update licence (3.1) and start trial (3.2) popups.
   Current display conditions: pluginConfig.allowChangingOptInStatus && !optedIn
   

   • These use cases are not available for cloud users. Users can change the Subscription level but they do so through Cloud administration UI.

4. The Stack Management > Advanced Settings section, where we can opt out of telemetry data collection.
   

   • Out of scope. The telemetry configuration section is not available in cloud, and if it were, we should show the privacy statement link.

❓Questions for @alexfrancoeur and @arisonl

• For (1), do we want to get rid of the privacy statement link and use plain text instead? or we want to get rid of the whole paragraph?
• Also for (1), should this paragraph depend on the cloud plugin (cloud.isCloudEnabled)? or could we simply use allowChangingOptInStatus as we do for (2)?
  • A) If you are fine with both options, then perhaps we could stick with allowChangingOptInStatus. This parameter already drives the behavior of the notice banner (2), and it would save us from adding one extra dependency with cloud. Risks:
    • The privacy statement message on (1) and (2) could appear again if we enable this parameter in cloud in the future.
    • ⚠ The privacy statement message will NOT appear for on-prem installations that are configured to not be able to opt out.
  • B) On the other hand, we could rely solely on cloud.isCloudEnabled. In that case, the message would never appear in cloud, despite the value of allowChangingOptInStatus. If we do, it probably makes sense to update (2) to use this parameter too, for the sake of consistency.
  • C) Or yet another option, we could make the privacy statement messages on (1) and (2) depend on both parameters.

Whilst waiting for feedback from @alexfrancoeur and @arisonl, I will create a PR to tackle (1) using strategy (B).

[alexfrancoeur]

Hey @gsoldevila, thanks for the detailed summary and questions. Generally, we do not need to reference any telemetry collection in Cloud because end users have already agreed to product usage collection in our EULA when signing up for our service. I believe that originally, all text referencing collection was hidden in Cloud and that there was a regression that exposed it within the interstitial page. With that said, I'll answer questions directly below.

For (1), do we want to get rid of the privacy statement link and use plain text instead? or we want to get rid of the whole paragraph?

We want to get rid of the whole paragraph in Cloud only. It is not necessary and generally causes confusion as there is no way to disable it.

Also for (1), should this paragraph depend on the cloud plugin (cloud.isCloudEnabled)? or could we simply use allowChangingOptInStatus as we do for (2)?

I'm not quite certain which option is best, but we want to hide any reference to telemetry in Cloud. Operational and product usage collection is agreed upon when accepting the EULA. I would think we should be consistent about our approach to disabling in Cloud. Whichever option is best suited for those two requirements works for me 😄

A) If you are fine with both options, then perhaps we could stick with allowChangingOptInStatus. This parameter already drives the behavior of the notice banner (2), and it would save us from adding one extra dependency with cloud.

I am fine with the two risks shared unless others have strong objections. There is no reason for Cloud to enable this configuration and if they do, it's a quick fix. As for an on-premise installation, I would not expect this setting to be configured either. If it is, there is a good chance the administrator read the documentation and the outcome was expected.

B) On the other hand, we could rely solely on cloud.isCloudEnabled. In that case, the message would never appear in cloud, despite the value of allowChangingOptInStatus. If we do, it probably makes sense to update (2) to use this parameter too, for the sake of consistency.

Showing and hiding these values based on whether or not the deployment is on Cloud should be the outcome we're looking for. The actual configuration used is less important to me as long as we standardize on one. What would be interesting to understand is if whether or not other "Cloud only experiences" are dependent on this flag. If there are more using the cloud.isCloudEnabled flag, maybe it make sense for this messaging to also standardize on it.

C) Or yet another option, we could make the privacy statement messages on (1) and (2) depend on both parameters.

I'd be open to this approach too but am not sure I fully understand the benefits for supporting both parameters.