update ticket received log line to be summary of tickets #2907
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2907 +/- ##
===================================================
+ Coverage 56.51727% 56.54162% +0.02435%
===================================================
Files 89 89
Lines 19456 19460 +4
===================================================
+ Hits 10996 11003 +7
+ Misses 7852 7849 -3
Partials 608 608
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
| @@ -198,6 +198,8 @@ func (orch *orchestrator) ProcessPayment(ctx context.Context, payment net.Paymen | |||
| ev := ticket.EV() | |||
| orch.node.Balances.Credit(sender, manifestID, ev) | |||
| totalEV.Add(totalEV, ev) | |||
| totalFaceValue.Add(totalFaceValue, ticket.FaceValue) | |||
| totalWinProb.Add(totalWinProb, ticket.WinProbRat()) | |||
There was a problem hiding this comment.
- So, we never set those to
0again and the log will always show the cumulative tickets? - I wonder how much sense it takes to add the probabilities and face values together? What this information actually may give you?
There was a problem hiding this comment.
- These variables are initialized in the ProcessPayment function so start at 0 every time. The log line is intended to show the cumulative amounts for the tickets (face value/win prob/ev).
- To me, this is a debug log line so more info is likely better right?
There was a problem hiding this comment.
Ahh, you're right. I missed that part that they are initialized for each payment processing. Thansk for the explanation!
| @@ -198,6 +198,8 @@ func (orch *orchestrator) ProcessPayment(ctx context.Context, payment net.Paymen | |||
| ev := ticket.EV() | |||
| orch.node.Balances.Credit(sender, manifestID, ev) | |||
| totalEV.Add(totalEV, ev) | |||
| totalFaceValue.Add(totalFaceValue, ticket.FaceValue) | |||
| totalWinProb.Add(totalWinProb, ticket.WinProbRat()) | |||
There was a problem hiding this comment.
Ahh, you're right. I missed that part that they are initialized for each payment processing. Thansk for the explanation!
|
Shouldn't |
I don't think the average gets you to the |
|
What does the sum of |
|
OK, see now. I was approaching from the reverse starting with EV / total win prob. I have no issue removing the winProb and faceValue totals. Was just trying to reduce the spam in logs from ticket processing on debug log level. Will get PR in at some point. |
|
i mean it's good to reduce logging, i just brought it up cause the change breaks this panel of mine (since there's no way to translate the sums back to a per-ticket basis with the new log format): |
What does this pull request do? Explain your changes. (required)
Moves
Receiving ticketlog line to be summary log line of tickets processed. Was a debug log line that printed for each ticket processed.Specific updates (required)
How did you test each of these updates (required)
@papabear99 ran similar version on mainnet
Does this pull request close any open issues?
No
Checklist:
makeruns successfully./test.shpass