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

Health API #272

Closed
xaur opened this issue Jun 10, 2021 · 8 comments · Fixed by #324
Closed

Health API #272

xaur opened this issue Jun 10, 2021 · 8 comments · Fixed by #324

Comments

@xaur
Copy link

xaur commented Jun 10, 2021

I suggest to discuss the addition of health metrics to existing endpoint like /vspinfo or create a new endpoint.

Two metrics to start with are votingwalletsonline and votingwalletstotal. I see the following uses for these fields:

  • when online is lower than the total, wallets could alert the user that there is an issue with the VSP
  • wallets could also present votingwalletstotal as one data point reflecting the reliability of a given VSP. iirc some dcrstakepool instances settled on the minimum required 3 wallets, while some added more have 5 or even 8 voting wallets
  • both motivations above are also valid for automated health monitor bots that could build reports or dashboards of the VSP ecosystem (see Status monitoring for Decred services decredcommunity/issues#148 for more on this)

This idea is inspired by one API I used that returned the count of shards online and shards total. These allowed me to alert the user that the data returned may be incomplete.

Extracted from #232.

@AlanBarber
Copy link

Great idea. I'd love to see more detailed reporting on the voting wallets that would include the version they are running. As a way of monitoring that the VSP is being actively maintained and upgrade in reasonable timeframes.

@ukane-philemon
Copy link
Contributor

Currently, the /vspinfo endpoint is only available to admins. Are we looking at opening it up to the public after adding these two new metrics? Discussion on making the /vspinfo public in issue #232 was not concluded. Quoting @jholdstock in issue #232

We could explore opening this up for public consumption.

Also are there more metrics to add? Quoting OP

Two metrics to start with are votingwalletsonline and votingwalletstotal.

I want to take this on.

@jholdstock
Copy link
Member

The /vspinfo endpoint is already public. The discussion in #232 was around making /status public, but that isn't feasible for the following reasons:

  • This endpoint queries dcrd and all voting wallets immediately, without caching, in order to provide admins with up to date information. That cannot be exposed publicly because it would be an easy DoS vector. Some kind of rate limiting or caching would be needed.
  • The call exposes too much information - eg. voting wallet IP addresses.
  • The API response depends entirely on the honesty of the VSP admin. For example, a dishonest admin could hard-code the response to say they have more wallets than they actually do.

We can tackle the first two by creating a new endpoint. I don't think there is anything we can do about the third problem.

@jholdstock
Copy link
Member

@AlanBarber - version is already exposed in /vspinfo. For example, https://testnet-vsp.jholdstock.uk/api/v3/vspinfo

"vspdversion":"1.1.0-pre"

@ukane-philemon
Copy link
Contributor

The /vspinfo endpoint is already public. The discussion in #232 was around making /status public, but that isn't feasible for the following reasons:

@jholdstock thanks for the feedback, had the /vspinfo endpoint in my head the whole time.

@AlanBarber
Copy link

@AlanBarber - version is already exposed in /vspinfo. For example, https://testnet-vsp.jholdstock.uk/api/v3/vspinfo

"vspdversion":"1.1.0-pre"

Sorry, guess I wasn't clear. I'm suggesting adding a votingwalletversions field that would return an array of the versions of all voting wallets.

To my surprise ./dcrctl --wallet version actually doesn't return the version of the dcrwallet but does give the versionstring of the dcrd it's connected to. There is a walletversion field that you see when you call ./dcrctl --wallet getinfo

{
  "version": 1070000,
  "protocolversion": 8,
  "walletversion": 1070000,
  ...
}

Anyways, more information is always useful and as a staker I need a way to know that my VSP is upgrading their service in a timely manner.

@jholdstock
Copy link
Member

@AlanBarber - vspd is configured to only run against certain versions of dcrwallet. For example, vspd 1.0.0 can only run against dcrwallet 1.6.X. vspd 1.1.0 can only run against dcrwallet 1.7.X. Therefore, checking the vspd version is enough to infer whether voting wallets have been upgraded or not.

vspd/rpc/dcrwallet.go

Lines 17 to 19 in 040ed56

var (
requiredWalletVersion = semver{Major: 8, Minor: 8, Patch: 0}
)

vspd/rpc/dcrwallet.go

Lines 89 to 95 in 040ed56

if !semverCompatible(requiredWalletVersion, sVer) {
log.Errorf("dcrwallet has incompatible JSON-RPC version (wallet=%s): got %s, expected %s",
c.String(), sVer, requiredWalletVersion)
failedConnections = append(failedConnections, connect.addr)
connect.Close()
continue
}

@ukane-philemon
Copy link
Contributor

@jholdstock are we looking at adding votingwalletstotal and votingwalletsonline to /vspinfo or create a new endpoint that would also include other vsp health details?

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 a pull request may close this issue.

4 participants