Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make OPA telemetry opt-in #489

Open
Tracked by #438
sbernauer opened this issue Nov 6, 2023 · 11 comments · Fixed by #487
Open
Tracked by #438

Make OPA telemetry opt-in #489

sbernauer opened this issue Nov 6, 2023 · 11 comments · Fixed by #487
Assignees
Labels

Comments

@sbernauer
Copy link
Member

sbernauer commented Nov 6, 2023

We want to have an opt-in flag at our OpaClusters that controls
--disable-telemetry disables anonymous information reporting (see: https://www.openpolicyagent.org/docs/latest/privacy)

@NickLarsenNZ
Copy link
Member

Should that be an opt-in thing? I can imagine most want to disable telemetry, but perhaps some want to enable it?

@sbernauer
Copy link
Member Author

Yeah, it should be an opt-in thing. We would need to add a CRD field and read that I guess. Changing the Issue to reflect that

@sbernauer sbernauer reopened this Nov 20, 2023
@sbernauer sbernauer changed the title Disable OPA telemetry Make OPA telemetry opt-in Nov 20, 2023
@lfrancke
Copy link
Member

I think we should just disable by default and have people use a config override for that.

Is that possible in this case?

@sbernauer
Copy link
Member Author

That's exactly what this Ticket wants. It's currently not possible to activate it, as we hard-code the start command

@lfrancke
Copy link
Member

lfrancke commented Dec 22, 2023

Sorry, I will clarify.

We would need to add a CRD field and read that I guess.

I think we should not add a CRD field for this if it is possible to set this using a config override.
Default: Telemetry disabled, can be enabled using config override (not a CRD field)

@sbernauer
Copy link
Member Author

Ah I see. https://www.openpolicyagent.org/docs/latest/privacy/ only mentions the CLI flag, but there might also be a config setting.

@NickLarsenNZ
Copy link
Member

Yeah, I dug through code and it doesn't look like there is a companion ENV var for it.

So if we explicitly disable it, and if someone really wants to enable, would they have to override the whole command? Or would we make an env var they can set to enable it and then adjust the command line based on it?

@sbernauer
Copy link
Member Author

https://github.com/stackabletech/opa-operator/blob/d3ad0da6ad4178b2ab79f449091a9fa2cbb000ab/rust/operator-binary/src/controller.rs#L864C15-L864C15 adds the parameter unconditional to the start command. You could (in theory) overwrite it with podOverrides but that's pain in the ***.
IMHO we should add a field such as spec.clusterConfig.telemetryEnabled which defaults to false and add --disable-telemetry conditionally.

@lfrancke
Copy link
Member

Ah! So do I understand this correctly that telemetry is already disabled now?

The PR says "Closes" this one but this one is not closed.

I assume this one is now about making it possible to enable telemetry again?

@sbernauer
Copy link
Member Author

Ah! So do I understand this correctly that telemetry is already disabled now?

Yes, you are right. Originally this issues was "Disable telemetry", PR did so and closed this Issue.

Than @NickLarsenNZ had the request, that users should be able to opt in, and we re-opened the ticket changed it to reflect the new requirements

@lfrancke
Copy link
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants