Skip to content

Conversation

bernie-laberge
Copy link
Contributor

SG-38210 Add RV Live Review metrics

This commit adds RV Live Review events to the allowlist in metrics.py

Context:
Note that prior to the RV 2024 release which was based off OpenRV as a submodule, RV had its own Amplitude secret to report Amplitude events. Since that we did not want to make this secret public when we open sourced RV, this secret was removed and we lost the capability of reporting Amplitude analytics events in RV.
That being said, the RV Code was only reporting Live Review related events.
This commit restores this functionality in RV.

This is just a proposal, please feel free to refuse this PR or ask me to rework it.

Thanks

Copy link
Contributor

@carlos-villavicencio-adsk carlos-villavicencio-adsk left a comment

Choose a reason for hiding this comment

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

Looks good. However, this will require an update also on SG web as well.

PS: To pass CI you can rebase #1010

@bernie-laberge
Copy link
Contributor Author

bernie-laberge commented Mar 24, 2025

Looks good. However, this will require an update also on SG web as well.

PS: To pass CI you can rebase #1010

OK thank you @carlos-villavicencio-adsk
Yes you are right : I also have a PR on the SG Web side. I've just added you as a reviewer.
I will wait until this one gets merged and rebase: #1010
Thanks!

@bernie-laberge bernie-laberge force-pushed the ticket/SG-38210_add_RV_live_review_to_metrics branch from 51b9800 to 643c11c Compare May 7, 2025 13:35
@bernie-laberge
Copy link
Contributor Author

FYI @carlos-villavicencio-adsk : I have rebased my branch with latest. Thanks

Copy link

codecov bot commented May 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.80%. Comparing base (e266c6e) to head (643c11c).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1014   +/-   ##
=======================================
  Coverage   79.80%   79.80%           
=======================================
  Files         198      198           
  Lines       20778    20779    +1     
=======================================
+ Hits        16582    16583    +1     
  Misses       4196     4196           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@julien-lang julien-lang left a comment

Choose a reason for hiding this comment

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

LGTM!

@bernie-laberge bernie-laberge merged commit b105eb0 into master May 7, 2025
25 checks passed
@bernie-laberge bernie-laberge deleted the ticket/SG-38210_add_RV_live_review_to_metrics branch May 7, 2025 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants