Skip to content

feat: Adds product analytics on most of the routes#307

Merged
frgfm merged 18 commits intomainfrom
telemetry
Jan 31, 2024
Merged

feat: Adds product analytics on most of the routes#307
frgfm merged 18 commits intomainfrom
telemetry

Conversation

@frgfm
Copy link
Copy Markdown
Member

@frgfm frgfm commented Nov 1, 2023

This PR introduces the following modifications:

  • simplifies the storage interactions (bucket/s3 --> storage.py)
  • adds a telemetry client
  • adds event capture on most routes

Any feedback is welcome!

cf. #304

@blenzi
Copy link
Copy Markdown

blenzi commented Nov 2, 2023

Thanks for this nice addition @frgfm ! I believe the errors in the CI come from
from app.services import telemetry_client (not defined) vs from app.services.telemetry import telemetry_client.

I would suggest monitoring also the alert notifications, in order to remove that from the db, as you suggested in #304.

@github-actions github-actions bot added the topic: docs Improvements or additions to documentation label Nov 12, 2023
Copy link
Copy Markdown

@blenzi blenzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for these additions, @frgfm !

I have no experience with PostHog but if I understand well, it will not have access to our db. Therefore, to maximize the possibilities it offers, we probably need some more detailed, human-readable information about the events. This will be required when analyzing individual alerts, statistics per site / group, etc. So I added in a few places the suggestion to include device / site / group name, updated information, etc. What do you think ?

Comment thread src/app/api/endpoints/sites.py Outdated
Comment thread src/app/api/endpoints/sites.py Outdated
Comment thread src/app/api/endpoints/alerts.py Outdated
Comment thread src/app/api/endpoints/alerts.py Outdated
Comment thread src/app/api/endpoints/devices.py Outdated
Comment thread src/app/api/endpoints/devices.py Outdated
Comment thread src/app/api/endpoints/devices.py Outdated
Comment thread src/app/api/endpoints/devices.py Outdated
Comment thread src/app/api/endpoints/devices.py
Comment thread src/app/api/endpoints/devices.py Outdated
@codecov
Copy link
Copy Markdown

codecov bot commented Jan 24, 2024

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (cf11dc7) 94.95% compared to head (1759f71) 94.59%.

Files Patch % Lines
src/app/api/endpoints/media.py 72.22% 5 Missing ⚠️
src/app/services/telemetry.py 73.68% 5 Missing ⚠️
src/app/services/services.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #307      +/-   ##
==========================================
- Coverage   94.95%   94.59%   -0.36%     
==========================================
  Files          63       63              
  Lines        1505     1592      +87     
==========================================
+ Hits         1429     1506      +77     
- Misses         76       86      +10     
Flag Coverage Δ
unittests 94.59% <91.91%> (-0.36%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@frgfm
Copy link
Copy Markdown
Member Author

frgfm commented Jan 24, 2024

So apart from "updated info" (feels like dumping the payload otherwise), I added everything that was immediately available (not requiring another DB operations)

@frgfm frgfm requested a review from blenzi January 24, 2024 10:36
Copy link
Copy Markdown

@blenzi blenzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @frgfm ! Let's start using it asap and adjust the level of information if needed

@frgfm frgfm merged commit 6b701dc into main Jan 31, 2024
@frgfm frgfm deleted the telemetry branch January 31, 2024 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants