ethereum-optimism / ethereum-optimism/optimism

go: the Big RPC cleanup ™ tracker

Open
#13,408 1 comment 0 reactions 0 assignees View on GitHub
C-cleanup go
Dominant language
Go
Stars
6.5k
Forks
4k
Avg merge
2d 15h
Merged PRs (30d)
145

Description

Change op-service metrics package:
- Remove deprecated (2 years ago...) `HTTPRequestDuration` from `PromHTTPRecorder`
- Rename `PromHTTPRecorder` to `HTTPServerMetrics`
- Turn `NewPromHTTPRecorder` into `MakeHTTPServerMetrics`
- Change `MakeHTTPServerMetrics` to take a `ns string, factory Factory` instead of registry and ns.
- Turn `HTTPRecorder` into `HTTPServerMetricer`
- Add a "rpc_domain" parameter to both the server and client metrics. When using multiple client connections, or multiple server endpoints, we can identify them this way. Instead of embedding as global metrics with a default name, we can embed as fields, and add methods on the service metrics struct to forward metering calls to the appropriate metrics recorder field.

Change Go services metrics:
- Add `HTTPServerMetrics` to the metrics struct of each service with an HTTP / HTTP-RPC server.
- Update all no-op metricers by embedding the no-op RPC / HTTP metricers.

Change op-geth:
- ~~Add a hooks option to the RPC server.~~
- ~~Add a hooks option to the RPC client.~~
- ~~Hooks to implement (both server and client use a `handler` instance that wraps a connection:~~ (recorder hooks have been added ✅ )
- ~~SetOpenRequests: `len(handler.respWait)` to monitor in-flight work, and any leaks.~~
- ~~SetOpenSubscriptions: `len(handler.clientSubs)` to monitor active subscriptions.~~
- ~~Wrap the `ServerCodec` interface to meter the requests (`readBatch` calls) and responses (`writeJSON` calls). Specifically, a hook interface that allows us to track the number of requests per method, and the number of errors / non-errors in responses per method.~~
- ~~Change `handle.handleCallMsg` to call the hook with the method and duration of a call. There are already logs that meter this, reuse those durations.~~
- Add `X_ETH_CHAIN_ID` header to HTTP responses.
- Http header option helper to add `X_ETH_CHAIN_ID` header to `rpc.Client` requests.
- Option on RPC client to run a call-back after a reconnect. Useful to check the chain ID of the endpoint we are connected to.

Change op-service `client` package:
- ~~Change `InstrumentedClient` to not manually instrument every method, but use the hooks of the op-geth RPC instead.~~ (done, rpc metrics via geth rpc recorder work now)
- Introduce a `client.WithChainIDCheck` function, that uses the `X_ETH_CHAIN_ID` headers to check if it's connected to the right RPC. If not HTTP, it should do a `eth_chainId` RPC request. This should be checked again whenever the client reconnects.

Change op-node:
- ~~Use the op-geth metrics hooks to attach metrics to the RPC server endpoints. Remove all manual RPC metering calls from the API endpoints.~~ (done)
- ~~Use the op-service RPC server package properly, instead of doing a different one-off RPC server setup.~~ done

Change op-signer (infra repo):
- This service is the only one who appears to use the current `PromHTTPRecorder`. It'll need to be updated to the new version.

Change Go services RPC server setups:
- add the websocket RPC server option everywhere. We can serve websocket RPC on the same port everywhere as we do the HTTP. It's just an upgrade-request.
- review RPC batch-limits and max request/response size on RPC servers
- - add the health endpoint RPC server option everywhere

Patterns:
- Naming:
- `-Metricer` for interfaces
- `-Metrics` for implementations
- `MakeMetrics` to return a struct. Avoid pointer indirection and interfaces; these things will be embedded into larger server metrics structs.
- RPC metrics everywhere
- HTTP server metrics everywhere
- Health endpoint everywhere
- Websocket support everywhere

Contributor guide

Open the contributing guide

Research direction

Start in the op-service metrics package and trace the HTTP/RPC metric interfaces and service metrics structs; then inspect the op-geth RPC client/server setup and the Go services listed in the tracker. Done means the named cleanup, chain-ID checking, websocket and health options, and consistent metrics coverage are completed across the affected services.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
api, backend, observability
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.