Skip to content
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

Add playback success rate to /api/v1/stats #4085

Merged
merged 4 commits into from
Nov 4, 2023

Conversation

syeopite
Copy link
Member

@syeopite syeopite commented Sep 1, 2023

#4083 (comment)

This PR tracks the total amount of /videoplayback requests and the ones that are successful in order to create a playback success rate in the /api/v1/stats endpoint.

By default it is an empty object:

"playback" : {}

But as soon as a video is played:

"playback": {
    "totalRequests": 19,
    "successfulRequests": 19,
    "ratio": 1.0
}

The caveat however is that with the way I have it currently set up the playback object gets reset back to an empty object each time the stats is refreshed. This is to prevent a success rate so high that 403 requests won't be able to budge the ratio at all. A better solution is probably needed there.

Closes #3957

@syeopite syeopite requested a review from a team as a code owner September 1, 2023 19:35
@syeopite syeopite requested review from unixfox and removed request for a team September 1, 2023 19:35
@syeopite syeopite marked this pull request as draft September 1, 2023 19:41
@syeopite syeopite marked this pull request as ready for review September 14, 2023 00:19
@unixfox unixfox requested a review from SamantazFox October 9, 2023 16:18
@unixfox
Copy link
Member

unixfox commented Oct 9, 2023

The caveat however is that with the way I have it currently set up the playback object gets reset back to an empty object each time the stats is refreshed. This is to prevent a success rate so high that 403 requests won't be able to budge the ratio at all. A better solution is probably needed there.

Not even sure if we have a better solution than that? @SamantazFox thoughts?

I think this PR is critical if we want to show to our users which instance works correctly or not.

@syeopite syeopite added the need-testing This feature needs to be deployed and tested to see if it's working, and doesn't break something label Oct 24, 2023
@unixfox
Copy link
Member

unixfox commented Nov 3, 2023

Very strange on my instance the stats are not getting updated, it stay at:

"playback" : {}

@syeopite
Copy link
Member Author

syeopite commented Nov 4, 2023

Huh that is weird. I can't replicate this on my end.

The problem is probably something with the get_playback_statistics function. Either it's not being called or it fails to update the "playback" value in the STATISTICS constant for whatever reason.

@unixfox
Copy link
Member

unixfox commented Nov 4, 2023

Oops I think I had an issue when testing the PR, it wasn't sending the requests to local /videoplayback endpoint.

Now it works:

"playback":{"totalRequests":3,"successfulRequests":0,"ratio":0.0}

But the stats do not stay for very long. Like after 10 seconds they are gone. But I think on a public instance it's ok as they are constantly people sending requests to the /videoplayback endpoint.

@syeopite
Copy link
Member Author

syeopite commented Nov 4, 2023

That can be mitigated if we bump the default stats refresh interval from one minute to something higher

@unixfox
Copy link
Member

unixfox commented Nov 4, 2023

That can be mitigated if we bump the default stats refresh interval from one minute to something higher

Could you do this change in this PR? Set it to more than 1 minute.

@syeopite
Copy link
Member Author

syeopite commented Nov 4, 2023

I've bumped it to ten minutes

@unixfox unixfox merged commit 438467f into iv-org:master Nov 4, 2023
7 checks passed
@unixfox
Copy link
Member

unixfox commented Nov 4, 2023

I have merged this PR because I consider it working. And we can always improve the PR later.

Also, this is really useful for the project https://github.com/iv-org/smart-ipv6-rotator

@unixfox unixfox removed the need-testing This feature needs to be deployed and tested to see if it's working, and doesn't break something label Nov 4, 2023
@unixfox
Copy link
Member

unixfox commented Nov 4, 2023

Now that I think of it. Wouldn't have been better to have a percentage instead of a ratio?

Percentage is much more appealing than a "ratio".

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.

[Feature request] API endpoint that informs if the instance has been blocked by Youtube
2 participants