debug api basic and full routing #1358
Conversation
janos
added
ready for review
pkg/debugapi/debugapi.go
Outdated
| @@ -30,33 +31,32 @@ import ( | |||
|
|
|||
| type Service interface { | |||
| http.Handler | |||
| Configure(overlay swarm.Address, publicKey, pssPublicKey ecdsa.PublicKey, ethereumAddress common.Address, p2p p2p.DebugService, pingpong pingpong.Interface, topologyDriver topology.Driver, storer storage.Storer, tags *tags.Tags, accounting accounting.Interface, settlement settlement.Interface, chequebookEnabled bool, swap swap.ApiInterface, chequebook chequebook.Service) | |||
There was a problem hiding this comment.
As explained in PR description, this interface should be removed for simplification and to avoid to have this method here.
|
Hey Janos: This is related: #1295 Can that be completed after your PR? |
pkg/debugapi/debugapi.go
Outdated
| // to expose /health endpoint, Go metrics and pprof. It is useful to expose | ||
| // these endpoints before all dependencies are configured and injected to have | ||
| // access to basic debugging tools and /health endpoint. | ||
| func New(logger logging.Logger, tracer *tracing.Tracer, corsAllowedOrigins []string) Service { |
There was a problem hiding this comment.
Minor suggestion: I think that overlay swarm.Address, publicKey, pssPublicKey ecdsa.PublicKey, ethereumAddress common.Address can already be set in this constructor as they are static and not expected to change during runtime.
There was a problem hiding this comment.
That is a very good suggestion. The only problem is ethereumAddress common.Address which needs to wait for InitChain function to complete. I am suggesting to have explicit SetEthereumAddress method and move other address arguments to constructor as you suggested. The EthereumAddress would require lock, but that is not so drastic.
This way there would be a debug api available as soon as possible for debugging and health and also with /addresses endpoint exposed, but ethereum address would be set only if and after InitChain is done. This would address some of the points in #1295 that @Eknir mentioned. WTYT @acud @Eknir?
There was a problem hiding this comment.
Actually, I don't think that you need InitChain for this. You can have the ethereum address from the signer which is passed to NewBee, exactly as it is used in InitChain
|
PR is updated to expose The documentation should be updated with the new behaviour after this PR is merged. Underlay addresses array in |
This PR introduces a two step Debug API router configuration.
Currently, debug api http listener is exposed at the end of the node starting process, when all of the services are constructed. This is a problem if there is a long running process, like localstore migration, and the /health endpoint is not exposed for a long time for kubernetes health check. This PR creates a debug api listener at the start of node initialization, exposing /health, /addresses and some other available debug endpoints, like metrics, pprof and vars. After all of the debug api dependencies are constructed, debug api service is configured with them and a full router is set.
This PR is with the minimal set of changes for this to be accomplished. A followup PR can be done to remove debugapi.Service interface and expose server in that package to avoid Configure method to be in the interface also. The interface is not needed. This change would require to unexport fields in the server struct which would raise the number of changed lines significantly here. I am avoiding that.