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

Init winning ticket value redeemed stat on boot #2916

Merged
merged 2 commits into from
Dec 11, 2023

Conversation

stronk-dev
Copy link
Contributor

What does this pull request do? Explain your changes. (required)

So before #2888 got merged, the winning ticket metric wasn't working properly. At least, when graphing the metric using livepeer_winning_tickets_recv you could see that it was counting them tickets, but when querying the increase over time (ie using sum by (datacenter)(increase(livepeer_winning_tickets_recv[7d]))) it would be short one or more tickets.

On every restart of the node the prometheus counters and gauges get reset. So using increase is the way to correctly display an increasing counter of tickets across restarts of the node

I am currently seeing the same behaviour with livepeer_value_redeemed
image
where on the first winning ticket, it counts it as an increase of 0, but then the next ticket gets counted correctly

Here is my theory: when the node boots, not all parameters have their values initialised to 0. It seemd that increase does not count an increase from null -> 1, but it does count an increase from 0 -> 1

This PR initialised the value_redeemed stat to 0 on boot, which should fix the issue i am seeing

How did you test each of these updates (required)
Not yet tested, deploying now...

Copy link

codecov bot commented Dec 8, 2023

Codecov Report

Merging #2916 (fb42082) into master (5b69cd6) will decrease coverage by 0.01340%.
Report is 2 commits behind head on master.
The diff coverage is 100.00000%.

Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                 @@
##              master       #2916         +/-   ##
===================================================
- Coverage   56.54162%   56.52822%   -0.01340%     
===================================================
  Files             89          89                 
  Lines          19460       19454          -6     
===================================================
- Hits           11003       10997          -6     
  Misses          7849        7849                 
  Partials         608         608                 
Files Coverage Δ
common/readfromfile.go 85.00000% <100.00000%> (-3.88889%) ⬇️
monitor/census.go 63.33596% <100.00000%> (+0.02887%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fdda451...fb42082. Read the comment docs.

Files Coverage Δ
common/readfromfile.go 85.00000% <100.00000%> (-3.88889%) ⬇️
monitor/census.go 63.33596% <100.00000%> (+0.02887%) ⬆️

@rickstaa
Copy link
Member

rickstaa commented Dec 8, 2023

This will fix #2917.

@FranckUltima
Copy link

Thank you! So happy to see this point finally resolved! I don’t know how long I spent on Grafana trying to find the query that would give me the exact number of winning tickets, even after a server restart, without ever succeeding.

@stronk-dev
Copy link
Contributor Author

Thank you! So happy to see this point finally resolved! I don’t know how long I spent on Grafana trying to find the query that would give me the exact number of winning tickets, even after a server restart, without ever succeeding.

Lol I know how you feel, when @papabear99 PR was merged it fixed the livepeer_winning_tickets_recv as an unintended side effect, so just applying the same logic to livepeer_value_redeemed now. we can finally accurately track work performed vs payouts redeemed

rickstaa added a commit to transcodeninja/livepeer-setup that referenced this pull request Dec 10, 2023
This commit ensures that the pending fees is displayed correctly. The
0.012 needed to be added for my orchestrator because of [this upstream
  bug](livepeer/go-livepeer#2916).
Copy link
Contributor

@thomshutt thomshutt 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, thanks @stronk-dev!

@thomshutt thomshutt merged commit f6bd15a into master Dec 11, 2023
15 checks passed
@thomshutt thomshutt deleted the md/bugfix/ValueRedeemedStat branch December 11, 2023 13:43
eliteprox pushed a commit to eliteprox/go-livepeer that referenced this pull request Feb 21, 2024
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.

4 participants