Skip to content
This repository has been archived by the owner on Aug 2, 2021. It is now read-only.

swap: added cheques metrics #2023

Merged
merged 4 commits into from
Dec 11, 2019
Merged

swap: added cheques metrics #2023

merged 4 commits into from
Dec 11, 2019

Conversation

holisticode
Copy link
Contributor

This PR adds metrics for cheques monitoring to the existing metrics.

Copy link
Contributor

@mortelli mortelli left a comment

Choose a reason for hiding this comment

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

no blockers.

maybe we can add something like a cashed cheques metric as well, no?

swap/peer.go Outdated Show resolved Hide resolved
swap/peer.go Outdated Show resolved Hide resolved
swap/swap.go Outdated Show resolved Hide resolved
@holisticode
Copy link
Contributor Author

@mortelli we could, but as currently the cheques are cashed right away, I think it may be enough for now to keep just received cheques - until we don't change cashing strategy, it should essentially be the same no?

@ralph-pichler
Copy link
Member

@holisticode Even in the current implementation we don't cash every cheque right away, so there is already a difference. I'm not convinced that the number of cashed cheques alone is useful (except perhaps in the context of #2012, but that is different from metrics), but the total amount of honey cashed probably would be. (What does number of cashed cheques even mean in the context of cumulatives cheques? If I received 3 from the same peer and I cashed the first and the third, did I cash 2 or 3?)

@holisticode
Copy link
Contributor Author

Ok agree. Will add a cheques.cashed.honey metric

swap/swap.go Outdated Show resolved Hide resolved
@holisticode holisticode merged commit e834de0 into master Dec 11, 2019
@holisticode
Copy link
Contributor Author

Closes #2019

@holisticode holisticode mentioned this pull request Dec 11, 2019
@acud acud added this to the 0.5.5 milestone Jan 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants