Improve instance status report #2271
Conversation
posthog/tasks/status_report.py
Outdated
| def fetch_events_count_by_name(params: Tuple[Any, ...]) -> dict: | ||
| results = fetch_sql( | ||
| """ | ||
| SELECT event as name, COUNT(*) as count |
There was a problem hiding this comment.
Not sure if it's related to this PR, but maybe we should obfuscate the name if the user has selected to anonymize their data?
posthog/tasks/status_report.py
Outdated
| def fetch_events_count_by_name(params: Tuple[Any, ...]) -> dict: | ||
| results = fetch_sql( | ||
| """ | ||
| SELECT event as name, COUNT(*) as count |
There was a problem hiding this comment.
Unsure if it's related to this PR but wondering if we should obfuscate the event names if the users chose to anonymize their data?
There was a problem hiding this comment.
Please file an issue - there was no semantic change to the previous behavior here, I'd like to avoid ballooning this into a bigger deal.
| "period": {"start_inclusive": period_start.isoformat(), "end_inclusive": period_end.isoformat()}, | ||
| } | ||
|
|
||
| report["users_who_logged_in"] = [ | ||
| {"id": user.id, "distinct_id": user.distinct_id} | ||
| if user.anonymize_data | ||
| else {"id": user.id, "distinct_id": user.distinct_id, "first_name": user.first_name, "email": user.email} |
There was a problem hiding this comment.
I know this is unrelated, but adding it here as this PR is an improvement on this code. I don't think it ever makes sense to send the user's first name and email, we're just adding PII here that it'll be very hard to keep track of. With the distinct ID we should be able to get their name if they haven't anonymized their data.
Also maybe send the total number of registered users too?
There was a problem hiding this comment.
Please file a separate issue with a suggestion how to anonymize. :)
There was a problem hiding this comment.
Sounds good, I'll make a PR for this after this one is merged
Maybe, but I'd like to see the problem before 'fixing' it. I'll update the task to handle failures in a way that reaches us. |
macobo
force-pushed
the
status-report-enhancements
branch
from
4ec9543
to
331ae9b
Compare
Also discovered issue Instance status report task reports wrong numbers #2255
Related issue User property for self-hosted deployment host #2138
Fixes Instance status report task reports wrong numbers #2255
Checklist