[Telemetry] Add Viper-based User config backed by Firestore DB#5478
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new system for managing persistent, privacy-preserving user configurations, primarily to support telemetry. It establishes a unique, anonymous client identifier, stores user preferences (like telemetry opt-in status) in Firestore, and integrates these settings into the telemetry collection process to improve usage tracking while maintaining user privacy. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new pkg/config package to manage user-specific configurations, including a unique user ID and telemetry settings, persisted via Firestore. It also integrates this user ID into telemetry events within the pkg/telemetry package and updates Go module dependencies. The review highlights several critical issues: unhandled errors in unique ID generation, inefficient and potentially non-functional remote configuration loading, a security risk of exposing sensitive data by saving all Viper settings to Firestore, and the absence of timeouts for Firestore operations, which could lead to CLI hangs. Additionally, the current implementation inefficiently creates and closes Firestore clients for each operation.
SwarnaBharathiMantena
left a comment
There was a problem hiding this comment.
LGTM!
(Discussed offline.)
e07b3ea
into
GoogleCloudPlatform:develop
Overview
This PR adds a persistent, privacy-preserving Viper-based user config backed by Firestore and updates the telemetry collector to attach pseudonymous client installation IDs to events.
Key Changes
pkg/config/user_config.go):generateUniqueID()which creates a stable, 24-character hex hash based on the hostname and user to serve as an anonymous identifier while avoiding PII leaks.SaveToFirestore()to persist user configurations (likeuser_idandtelemetry_enabled) into a Firestore database in thehpc-toolkit-gscGCP project using the Firestore API.pkg/telemetry/collector.go):BuildConcordEvent()to includeClientInstallIdby fetching the ID from the persistent user config.pkg/telemetry/README.md):go.mod&go.sum):cloud.google.com/go/firestore,github.com/spf13/viper, and their related transitive dependencies.pkg/config/user_config_test.go&pkg/telemetry/collector_test.go):generateUniqueID()is deterministic and correctly hashed to a 24-character string.KINDLY NOTE THAT TELEMETRY DATA IS NOT BEING COLLECTED YET.