CONSOLE-3944: Disable segment analytics when cluster telemetry is disabled #13677

TheRealJon · 2024-03-15T14:53:33Z

Disable console-telemetry-plugin segment analytics when window.SERVER_FLAGS.telemetry.TELEMETER_CLIENT_DISABLED flag is set

openshift-ci-robot · 2024-03-15T14:53:37Z

@TheRealJon: This pull request references CONSOLE-3944 which is a valid jira issue.

In response to this:

Disable console telemetry listeners when telemeterClientDisabled flag is set

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

TheRealJon · 2024-03-15T17:17:43Z

/retest

TheRealJon · 2024-03-15T17:17:48Z

QE Approver
/assign @yapei

Docs Approver:
/assign @opayne1

PX Approver:
/assign @RickJWagner

RickJWagner · 2024-03-15T18:48:32Z

/label px-approved

jerolimov · 2024-03-18T10:29:46Z

/cc @jerolimov @jhadvig
/assign @jerolimov @jhadvig

opayne1 · 2024-03-20T14:57:29Z

/label docs-approved

…abled Disabe console telemetry listeners when `telemeterClientDisabled` flag is set

…handler, and log events that have been ignored.

TheRealJon · 2024-03-21T18:03:47Z

/retest

openshift-ci-robot · 2024-04-03T19:23:52Z

@TheRealJon: This pull request references CONSOLE-3944 which is a valid jira issue.

In response to this:

Disable console telemetry listeners when window.SERVER_FLAGS.telemetry.TELEMETER_CLIENT_DISABLED flag is set

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

yapei · 2024-04-07T06:16:19Z

frontend/packages/console-telemetry-plugin/src/listeners/segment.ts

@@ -110,12 +110,18 @@ export const eventListener: TelemetryEventListener = async (
  eventType: string,
  properties?: any,
 ) => {
-  if (TELEMETRY_DEBUG) {
+  if (TELEMETRY_DISABLED || !apiKey) {


Tested the changes and it looks like we have some issue here

confirm if telemeter client is disabled or not(based on latest code change, we check deployment/telemeter-client status)

$ oc get deployment telemeter-client -n openshift-monitoring NAME READY UP-TO-DATE AVAILABLE AGE telemeter-client 1/1 1 1 3h7m $ oc get cm console-config -n openshift-console | grep tele (nothing returned)

Enable demo plugin and do some navigation in the web, we can see messages in browser console

console-telemetry-plugin: telemetry disabled - ignoring telemetry event xxxx

based on the data in step1 we can see that telemeter client is NOT disabled, so it looks a bit wired we are printing telemetry disabled message

However after I disabled telemeter client, browser console prints following message(when telemeter is actually disabled, we didn't print correct message)

confirm telemeter is actually disabled

$ oc get cm console-config -n openshift-console -o yaml | grep telemetry -A2 telemetry: TELEMETER_CLIENT_DISABLED: "true" kind: ConfigMap $ oc get deployment telemeter-client -n openshift-monitoring Error from server (NotFound): deployments.apps "telemeter-client" not found

Check browser console message, this time we didn't print telemetry disabled - ignoring telemetry event xxx message

There may be some confusion about the scope of this change. This PR is only meant to disable Segment analytics when cluster telemetry is disabled in the cluster monitoring operator. It will not disable all telemetry in the UI.

Additionally, other conditions will cause Segment analytics to be disabled. Could you check these four conditions to ensure we shouldn't show the disabled message:

window.SERVER_FLAGS?.telemetry?.DISABLED === 'false'

window.SERVER_FLAGS?.telemetry?.DEVSANDBOX_DISABLED === 'false'

window.SERVER_FLAGS?.telemetry?.TELEMETER_CLIENT_DISABLED === 'false'

window.SERVER_FLAGS?.telemetry?.DEVSANDBOX_SEGMENT_API_KEY or window.SERVER_FLAGS?.telemetry?.SEGMENT_API_KEY is defined

Hi @yapei , please take a look how I've tested the changes below. Maybe this is enough to get your qe-approve label.

The problem I've noticed with the telemeter-client Deployment is, that it was automatically removed again. I expect that the monitoring-operator reconciler removed it. A better test might be to enable telemetry on a cluster bot but I don't know how to enable it.

Oh, that was easier then expected. Instead of creating the telemeter-client Deployment manually, I just enabled telemetry in the cluster-monitoring-config ConfigMap in the openshift-monitoring namespace.

After that you must wait a bit and reload the web console.

https://access.redhat.com/documentation/de-de/openshift_container_platform/4.1/html/telemetry/opting-out-of-telemetry

Peek.2024-04-16.11-07.mp4

jerolimov

Verified this change on a cluster bot:

Works as expected. I personally would recommend a small change on the operator side:

Allow cluster admins to override TELEMETRY_CLIENT_DISABLED with an annotation! Currently, there is no option to enable segment analytics if the telemeter-client Deployment isn't available because TELEMETRY_CLIENT_DISABLED is then always true.

@jhadvig maybe you can apply that if you touch the operator again.

Here are two long recordings how I've tested the change:

Peek.2024-04-16.10-49.mp4

Peek.2024-04-16.10-54.mp4

Peek.2024-04-16.11-07.mp4

/lgtm

openshift-ci · 2024-04-16T09:02:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jerolimov, TheRealJon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~frontend/packages/console-telemetry-plugin/OWNERS~~ [TheRealJon,jerolimov]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

TheRealJon · 2024-04-16T20:52:26Z

frontend/packages/console-telemetry-plugin/src/listeners/segment.ts

@@ -110,12 +110,18 @@ export const eventListener: TelemetryEventListener = async (
  eventType: string,
  properties?: any,
 ) => {
-  if (TELEMETRY_DEBUG) {
+  if (TELEMETRY_DISABLED || !apiKey) {


There might also be some confusion about this conditional block because there is no case where it will evaluate to true if TELEMETRY_DISABLED === true. This entire extension will not be resolved in that case. See the following:

Extension definition, dependent on TELEMETRY flag

Flag definition, dependent on TELEMETRY_DISABLED flag

TheRealJon · 2024-04-16T21:00:28Z

frontend/packages/console-telemetry-plugin/src/listeners/segment.ts

@@ -110,12 +110,18 @@ export const eventListener: TelemetryEventListener = async (
  eventType: string,
  properties?: any,
 ) => {
-  if (TELEMETRY_DEBUG) {
+  if (TELEMETRY_DISABLED || !apiKey) {


I don't want to add further changes to this PR since it's already got an lgtm, but I might follow on with a tweak like this:

Suggested change

if (TELEMETRY_DISABLED || !apiKey) {

if (!apiKey) {

// eslint-disable-next-line no-console

console.debug(

'console-telemetry-plugin: segment API key missing - ignoring telemetry event:',

eventType,

properties,

);

return;

yapei · 2024-04-17T00:58:26Z

Thanks @jerolimov @TheRealJon except this issue everything looks good in my testing
/label qe-approved

openshift-ci-robot · 2024-04-17T00:58:32Z

@TheRealJon: This pull request references CONSOLE-3944 which is a valid jira issue.

In response to this:

Disable console telemetry listeners when window.SERVER_FLAGS.telemetry.TELEMETER_CLIENT_DISABLED flag is set

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2024-04-17T03:44:58Z

/retest-required

Remaining retests: 0 against base HEAD f666ce3 and 2 for PR HEAD 5f6dc89 in total

jerolimov · 2024-04-17T08:02:42Z

/retest

openshift-ci-robot · 2024-04-17T13:42:38Z

@TheRealJon: This pull request references CONSOLE-3944 which is a valid jira issue.

In response to this:

Disable console-telemetry-plugin segment analytics when window.SERVER_FLAGS.telemetry.TELEMETER_CLIENT_DISABLED flag is set

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

TheRealJon · 2024-04-17T13:51:47Z

/retest

openshift-ci-robot · 2024-04-17T21:39:05Z

/retest-required

Remaining retests: 0 against base HEAD 7b8d108 and 1 for PR HEAD 5f6dc89 in total

openshift-ci · 2024-04-18T02:26:31Z

@TheRealJon: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Mar 15, 2024

openshift-ci bot requested review from divyanshiGupta and karthikjeeyar March 15, 2024 14:54

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 15, 2024

openshift-ci bot assigned opayne1, RickJWagner and yapei Mar 15, 2024

RickJWagner removed their assignment Mar 15, 2024

openshift-ci bot added the px-approved Signifies that Product Support has signed off on this PR label Mar 15, 2024

openshift-ci bot assigned jerolimov Mar 18, 2024

openshift-ci bot requested a review from jerolimov March 18, 2024 10:30

openshift-ci bot assigned jhadvig Mar 18, 2024

openshift-ci bot requested a review from jhadvig March 18, 2024 10:30

openshift-ci bot added component/core Related to console core functionality component/shared Related to console-shared labels Mar 20, 2024

openshift-ci bot added the docs-approved Signifies that Docs has signed off on this PR label Mar 20, 2024

TheRealJon added 2 commits March 20, 2024 13:41

CONSOLE-3944: Disable segment analytics when cluster telemetry is dis…

b1fa041

…abled Disabe console telemetry listeners when `telemeterClientDisabled` flag is set

Check for TELEMETRY_DISABLED before TELEMETRY_DEBUG in segment event …

f4e140d

…handler, and log events that have been ignored.

TheRealJon force-pushed the CONSOLE-3944 branch from 2c140d2 to f4e140d Compare March 20, 2024 17:41

Rename telemeterClientDisabled flag to match UPPERCASE convention

5f6dc89

yapei reviewed Apr 7, 2024

View reviewed changes

jerolimov approved these changes Apr 16, 2024

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 16, 2024

TheRealJon commented Apr 16, 2024

View reviewed changes

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Apr 17, 2024

openshift-merge-bot bot merged commit 8ef3b38 into openshift:master Apr 18, 2024
6 checks passed

TheRealJon deleted the CONSOLE-3944 branch April 18, 2024 14:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CONSOLE-3944: Disable segment analytics when cluster telemetry is disabled #13677

CONSOLE-3944: Disable segment analytics when cluster telemetry is disabled #13677

TheRealJon commented Mar 15, 2024 •

edited

openshift-ci-robot commented Mar 15, 2024 •

edited by openshift-ci bot

TheRealJon commented Mar 15, 2024

TheRealJon commented Mar 15, 2024

RickJWagner commented Mar 15, 2024

jerolimov commented Mar 18, 2024 •

edited

opayne1 commented Mar 20, 2024

TheRealJon commented Mar 21, 2024

openshift-ci-robot commented Apr 3, 2024 •

edited by openshift-ci bot

yapei Apr 7, 2024

yapei Apr 7, 2024 •

edited

TheRealJon Apr 9, 2024

jerolimov Apr 16, 2024

jerolimov Apr 16, 2024 •

edited

jerolimov left a comment •

edited

openshift-ci bot commented Apr 16, 2024

TheRealJon Apr 16, 2024 •

edited

TheRealJon Apr 16, 2024

yapei commented Apr 17, 2024

openshift-ci-robot commented Apr 17, 2024 •

edited by openshift-ci bot

openshift-ci-robot commented Apr 17, 2024

jerolimov commented Apr 17, 2024

openshift-ci-robot commented Apr 17, 2024 •

edited by openshift-ci bot

TheRealJon commented Apr 17, 2024

openshift-ci-robot commented Apr 17, 2024

openshift-ci bot commented Apr 18, 2024

-  if (TELEMETRY_DISABLED || !apiKey) {
+  if (!apiKey) {
+    // eslint-disable-next-line no-console
+    console.debug(
+      'console-telemetry-plugin: segment API key missing - ignoring telemetry event:',
+      eventType,
+      properties,
+    );
+    return;

CONSOLE-3944: Disable segment analytics when cluster telemetry is disabled #13677

CONSOLE-3944: Disable segment analytics when cluster telemetry is disabled #13677

Conversation

TheRealJon commented Mar 15, 2024 • edited

openshift-ci-robot commented Mar 15, 2024 • edited by openshift-ci bot

TheRealJon commented Mar 15, 2024

TheRealJon commented Mar 15, 2024

RickJWagner commented Mar 15, 2024

jerolimov commented Mar 18, 2024 • edited

opayne1 commented Mar 20, 2024

TheRealJon commented Mar 21, 2024

openshift-ci-robot commented Apr 3, 2024 • edited by openshift-ci bot

yapei Apr 7, 2024

Choose a reason for hiding this comment

yapei Apr 7, 2024 • edited

Choose a reason for hiding this comment

TheRealJon Apr 9, 2024

Choose a reason for hiding this comment

jerolimov Apr 16, 2024

Choose a reason for hiding this comment

jerolimov Apr 16, 2024 • edited

Choose a reason for hiding this comment

jerolimov left a comment • edited

Choose a reason for hiding this comment

openshift-ci bot commented Apr 16, 2024

TheRealJon Apr 16, 2024 • edited

Choose a reason for hiding this comment

TheRealJon Apr 16, 2024

Choose a reason for hiding this comment

yapei commented Apr 17, 2024

openshift-ci-robot commented Apr 17, 2024 • edited by openshift-ci bot

openshift-ci-robot commented Apr 17, 2024

jerolimov commented Apr 17, 2024

openshift-ci-robot commented Apr 17, 2024 • edited by openshift-ci bot

TheRealJon commented Apr 17, 2024

openshift-ci-robot commented Apr 17, 2024

openshift-ci bot commented Apr 18, 2024

TheRealJon commented Mar 15, 2024 •

edited

openshift-ci-robot commented Mar 15, 2024 •

edited by openshift-ci bot

jerolimov commented Mar 18, 2024 •

edited

openshift-ci-robot commented Apr 3, 2024 •

edited by openshift-ci bot

yapei Apr 7, 2024 •

edited

jerolimov Apr 16, 2024 •

edited

jerolimov left a comment •

edited

TheRealJon Apr 16, 2024 •

edited

openshift-ci-robot commented Apr 17, 2024 •

edited by openshift-ci bot

openshift-ci-robot commented Apr 17, 2024 •

edited by openshift-ci bot