-
-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Stat Collector #1007
base: main
Are you sure you want to change the base?
Stat Collector #1007
Conversation
… the amplipi folder, importing it in app.py, and running it in the same venv as core amplipi
@api.get('/collected_stats') | ||
@api.get('/collected_stats/') | ||
def get_collected_stats(): | ||
"""Get the current contents of statcollector output file after filtering out any empty stream data""" | ||
survey = dict(statcollector.UsageSurveySchema.load_from_disk()) | ||
return {header: data for header, data in survey.items() if data != statcollector.StreamUsageSchema()} | ||
|
||
|
||
@api.post('/collected_stats') | ||
@api.post('/collected_stats/') | ||
def post_collected_stats(): | ||
"""Send the contents of the statcollector output file to AmpliPi devs""" | ||
survey = statcollector.UsageSurveySchema.load_from_disk() | ||
survey.phone_home() | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These do not belong at the top of this file
I do not know where they belong, but it isn't the top of the top
They stay here in the meantime because it's easier to not need to scroll too much to edit imports if need be during devwork
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These stats are effectively metadata about the api. Putting them in the API feels weird. That said keeping them outside of the api may make accessing them a little more awkward.
if len(matches) != 0: # If there is at least one playing match, calculate runtime | ||
self.current_runtime = self.current_runtime + timediff.seconds | ||
|
||
self.peak_runtime = max(self.peak_runtime, self.current_runtime) if self.current_runtime < 86400 else 86400 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no real reason to limit this to 24 hours, I just had the feeling that more than 24 hours would lead to outliers and this is a method of negating that. It might be worthwhile to remove the limit just so we can see what sort of lifespan that long-lived stream errors depend on
What does this change intend to accomplish?
Add an optional user statistics survey to help us decide what to spend our devtime on, and potentially help find bugs and improve support
Currently this is a draft that only contains the script that collects the stats, this will eventually contain a good means of invoking the script, a means of sending the data upstream to home base, and a portion of the UI that allows a user to activate the functionality in general
Checklist
./scripts/test