Cog 597 refactor analytics #231
Conversation
Added proxy usage with vercel hosting for telemetry and analytics Feature COG-597
Adding user_id to event properties allows tracking of which user started the event Refactor COG-597
WalkthroughThe pull request introduces modifications to the Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 13947935 | Triggered | Generic High Entropy Secret | d90f5fe | cognee/shared/utils.py | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
cognee/shared/utils.py (3)
20-20: Consider makingvercel_urlconfigurableTo enhance flexibility and security, it's recommended to avoid hardcoding the
vercel_url. Instead, retrieve it from an environment variable or a configuration file. This allows for easier updates and environment-specific configurations.Apply this diff to make the URL configurable:
- vercel_url = "https://proxyanalytics.vercel.app" + vercel_url = os.getenv("ANALYTICS_PROXY_URL", "https://proxyanalytics.vercel.app")This change uses the
ANALYTICS_PROXY_URLenvironment variable if it's set, with a default fallback to the current URL.
51-51: Avoid duplication ofuser_idin payloadThe
user_idis included in bothuser_propertiesandproperties. To prevent redundancy and potential confusion, consider removing the duplicate entry unless both are required for the analytics proxy.Apply this diff to remove the duplicate:
payload = { "anonymous_id": str(get_anonymous_id()), "event_name": event_name, "user_properties": { - "user_id": str(user_id), }, "properties": { "time": current_time.strftime("%m/%d/%Y"), "user_id": str(user_id), **additional_properties }, }Alternatively, if
user_idis required inuser_properties, remove it fromproperties:payload = { "anonymous_id": str(get_anonymous_id()), "event_name": event_name, "user_properties": { "user_id": str(user_id), }, "properties": { "time": current_time.strftime("%m/%d/%Y"), - "user_id": str(user_id), **additional_properties }, }Ensure consistency based on how the analytics proxy expects the data.
Also applies to: 55-55
54-54: Use ISO 8601 format fortimefieldConsider formatting the
timefield using the ISO 8601 standard for better interoperability and parsing consistency across different systems.Apply this diff to update the time format:
- "time": current_time.strftime("%m/%d/%Y"), + "time": current_time.isoformat(),The
isoformat()method provides a standardized timestamp, such as'2023-10-21T15:03:00+00:00'.
| response = requests.post(vercel_url, json=payload) | ||
|
|
||
| try: | ||
| posthog.capture(get_anonymous_id(), event_name, properties) | ||
| except Exception as e: | ||
| print("ERROR sending telemetric data to Posthog. See exception: %s", e) | ||
| if response.status_code != 200: | ||
| print(f"Error sending telemetry through proxy: {response.status_code}") |
There was a problem hiding this comment.
Add exception handling and use logging instead of print
Wrap the HTTP request in a try-except block to handle potential exceptions like connection errors. Additionally, replace print statements with the logging module for better control over logging levels and outputs.
Apply this diff to improve error handling and logging:
+import logging
+logging.basicConfig(level=logging.INFO)
def send_telemetry(event_name: str, user_id, additional_properties: dict = {}):
if os.getenv("TELEMETRY_DISABLED"):
return
env = os.getenv("ENV")
if env in ["test", "dev"]:
return
current_time = datetime.now(timezone.utc)
payload = {
"anonymous_id": str(get_anonymous_id()),
"event_name": event_name,
"user_properties": {
"user_id": str(user_id),
},
"properties": {
"time": current_time.strftime("%m/%d/%Y"),
"user_id": str(user_id),
**additional_properties
},
}
- response = requests.post(vercel_url, json=payload)
- if response.status_code != 200:
- print(f"Error sending telemetry through proxy: {response.status_code}")
+ try:
+ response = requests.post(vercel_url, json=payload)
+ response.raise_for_status()
+ except requests.exceptions.RequestException as e:
+ logging.error(f"Error sending telemetry: {e}")
+ return
This change includes:
- Importing the
loggingmodule and configuring it. - Wrapping the
requests.postcall in atry-exceptblock to catch exceptions. - Using
response.raise_for_status()to check for HTTP errors. - Logging errors using
logging.error().
This approach ensures that exceptions are properly handled, and error messages are logged appropriately.
Committable suggestion skipped: line range outside the PR's diff.
Refactor telemetry analytics
Summary by CodeRabbit
New Features
Bug Fixes