Skip to content

Commit e6891a5

Browse files
authored
[Session] Code health and improvements to enable parallel requests (#340)
> [!NOTE] > 1. Make sure to read the TODO in `gateway/request_context.go` > 2. The code in `qos/selector/multiple_selection.go` was heavily "vibe coded" since this is "standard stuff" > 3. Since this is a large PR, please focus on ensuring there's no regression (or issues) on making requests ## Primary Changes - Lay the foundation for making parallel requests (with it hard-coded to 1 for now) - Introduce and implement `SelectMultiple` in every qos module - Various helpers for selecting and logging multiple endpoints with domain diversity ## Secondary changes - Code health & cleanup along the way - Misc QoL & code health improvements
1 parent 8ccdf16 commit e6891a5

40 files changed

+1592
-439
lines changed

data/legacy_protocol_shannon.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ func setLegacyFieldsFromShannonProtocolObservations(
8080

8181
// Extract the endpoint's domain from its URL.
8282
endpointDomain, err := shannonmetrics.ExtractDomainOrHost(endpointObservation.EndpointUrl)
83-
// Error extracting the endpoint domain: log the error.
8483
if err != nil {
8584
logger.With("endpoint_url", endpointObservation.EndpointUrl).Warn().Err(err).Msg("Could not extract domain from Shannon endpoint URL")
8685
return legacyRecord

gateway/errors.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,8 @@ var (
1010

1111
// QoS instance rejected the request.
1212
// e.g. HTTP payload could not be unmarshaled into a JSONRPC request.
13-
ErrGatewayRejectedByQoS = errors.New("QoS instance rejected the request")
13+
errGatewayRejectedByQoS = errors.New("QoS instance rejected the request")
14+
15+
// Error building protocol contexts from HTTP request.
16+
errBuildProtocolContextsFromHTTPRequest = errors.New("error building protocol contexts from HTTP request")
1417
)

gateway/gateway.go

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -60,18 +60,22 @@ type Gateway struct {
6060
// Reference: https://en.wikipedia.org/wiki/Template_method_pattern
6161
//
6262
// TODO_FUTURE: Refactor when adding other protocols (e.g. gRPC):
63-
// - Extract generic processing into common method
64-
// - Keep HTTP-specific details separate
65-
func (g Gateway) HandleServiceRequest(ctx context.Context, httpReq *http.Request, w http.ResponseWriter) {
66-
// build a gatewayRequestContext with components necessary to process requests.
63+
// - Extract generic processing into common method
64+
// - Keep HTTP-specific details separate
65+
func (g Gateway) HandleServiceRequest(
66+
ctx context.Context,
67+
httpReq *http.Request,
68+
responseWriter http.ResponseWriter,
69+
) {
70+
// Build a gatewayRequestContext with components necessary to process requests.
6771
gatewayRequestCtx := &requestContext{
6872
logger: g.Logger,
73+
context: ctx,
6974
gatewayObservations: getUserRequestGatewayObservations(httpReq),
7075
protocol: g.Protocol,
7176
httpRequestParser: g.HTTPRequestParser,
7277
metricsReporter: g.MetricsReporter,
7378
dataReporter: g.DataReporter,
74-
context: ctx,
7579
}
7680

7781
defer func() {
@@ -89,17 +93,22 @@ func (g Gateway) HandleServiceRequest(ctx context.Context, httpReq *http.Request
8993
// Determine the type of service request and handle it accordingly.
9094
switch determineServiceRequestType(httpReq) {
9195
case websocketServiceRequest:
92-
g.handleWebSocketRequest(ctx, httpReq, gatewayRequestCtx, w)
96+
g.handleWebSocketRequest(ctx, httpReq, gatewayRequestCtx, responseWriter)
9397
default:
94-
g.handleHTTPServiceRequest(ctx, httpReq, gatewayRequestCtx, w)
98+
g.handleHTTPServiceRequest(ctx, httpReq, gatewayRequestCtx, responseWriter)
9599
}
96100
}
97101

98-
// handleHTTPRequest handles a standard HTTP service request.
99-
func (g Gateway) handleHTTPServiceRequest(ctx context.Context, httpReq *http.Request, gatewayRequestCtx *requestContext, w http.ResponseWriter) {
102+
// handleHTTPServiceRequest handles a standard HTTP service request.
103+
func (g Gateway) handleHTTPServiceRequest(
104+
_ context.Context,
105+
httpReq *http.Request,
106+
gatewayRequestCtx *requestContext,
107+
responseWriter http.ResponseWriter,
108+
) {
100109
defer func() {
101110
// Write the user-facing HTTP response. This is deliberately not called for websocket requests as they do not return an HTTP response.
102-
gatewayRequestCtx.WriteHTTPUserResponse(w)
111+
gatewayRequestCtx.WriteHTTPUserResponse(responseWriter)
103112
}()
104113

105114
// TODO_TECHDEBT(@adshmh): Pass the context with deadline to QoS once it can handle deadlines.
@@ -117,7 +126,7 @@ func (g Gateway) handleHTTPServiceRequest(ctx context.Context, httpReq *http.Req
117126
// This will allow the QoS service to return more helpful diagnostic messages and enable better metrics collection for different failure modes.
118127
//
119128
// Build the protocol context for the HTTP request.
120-
err = gatewayRequestCtx.BuildProtocolContextFromHTTP(httpReq)
129+
err = gatewayRequestCtx.BuildProtocolContextsFromHTTPRequest(httpReq)
121130
if err != nil {
122131
return
123132
}
@@ -128,16 +137,21 @@ func (g Gateway) handleHTTPServiceRequest(ctx context.Context, httpReq *http.Req
128137
_ = gatewayRequestCtx.HandleRelayRequest()
129138
}
130139

131-
// handleWebsocketRequest handles WebSocket connection requests
132-
func (g Gateway) handleWebSocketRequest(ctx context.Context, httpReq *http.Request, gatewayRequestCtx *requestContext, w http.ResponseWriter) {
140+
// handleWebSocketRequest handles WebSocket connection requests.
141+
func (g Gateway) handleWebSocketRequest(
142+
_ context.Context,
143+
httpReq *http.Request,
144+
gatewayRequestCtx *requestContext,
145+
w http.ResponseWriter,
146+
) {
133147
// Build the QoS context for the target service ID using the HTTP request's payload.
134148
err := gatewayRequestCtx.BuildQoSContextFromWebsocket(httpReq)
135149
if err != nil {
136150
return
137151
}
138152

139153
// Build the protocol context for the HTTP request.
140-
err = gatewayRequestCtx.BuildProtocolContextFromHTTP(httpReq)
154+
err = gatewayRequestCtx.BuildProtocolContextsFromHTTPRequest(httpReq)
141155
if err != nil {
142156
return
143157
}

gateway/hydrator.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ func (eph *EndpointHydrator) performChecks(serviceID protocol.ServiceID, service
197197
serviceQoS: serviceQoS,
198198
qosCtx: serviceRequestCtx,
199199
protocol: eph.Protocol,
200-
protocolCtx: hydratorRequestCtx,
200+
protocolContexts: []ProtocolRequestContext{hydratorRequestCtx},
201201
// metrics reporter for exporting metrics on hydrator service requests.
202202
metricsReporter: eph.MetricsReporter,
203203
// data reporter for exporting data on hydrator service requests to the data pipeline.

gateway/qos.go

Lines changed: 67 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -9,103 +9,104 @@ import (
99
"github.com/buildwithgrove/path/protocol"
1010
)
1111

12-
// RequestQoSContext represents the interactions of the gateway with the QoS instance
13-
// corresponding to the service specified by a service request.
12+
// RequestQoSContext
1413
//
15-
// A RequestQoSContext can be built in various ways such as:
16-
// - 1. Building a new context by parsing an organic request from an end-user
17-
// - 2. Building a new context based on a desired endpoint check, e.g. an `eth_chainId` request on an EVM blockchain.
18-
// - 3. Rebuilding an existing context by deserializing a shared context from another PATH instance
14+
// Represents interactions between the gateway and the QoS instance for a given service request.
15+
//
16+
// Construction methods:
17+
// - Parse an organic request from an end-user.
18+
// - Rebuild from a shared context deserialized from another PATH instance.
1919
type RequestQoSContext interface {
20-
// TODO_TECHDEBT: This should eventually return a []Payload
21-
// to allow mapping a single RelayRequest into multiple ServiceRequests,
22-
// e.g. A single batch relay request on a JSONRPC blockchain should be decomposable into
23-
// multiple independent requests.
20+
// TODO_TECHDEBT: Should eventually return []Payload
21+
// - Allows mapping a single RelayRequest into multiple ServiceRequests.
22+
// - Example: A batch relay request on JSONRPC should decompose into multiple independent requests.
2423
GetServicePayload() protocol.Payload
2524

26-
// TODO_FUTURE: add retry-related return values to UpdateWithResponse,
27-
// or add retry-related methods to the interface, e.g. Failed(), ShouldRetry().
28-
// UpdateWithResponse is used to inform the request QoS context of the
29-
// payload returned by a specific endpoint in response to the service
30-
// payload produced (through the `GetServicePayload` method) by the
31-
// request QoS context instance
25+
// TODO_FUTURE:
26+
// - Add retry-related return values to UpdateWithResponse,
27+
// or add retry-related methods (e.g., Failed(), ShouldRetry()).
28+
//
29+
// UpdateWithResponse:
30+
// - Informs the request QoS context of the payload returned by a specific endpoint.
31+
// - Response is for the service payload produced by GetServicePayload.
3232
UpdateWithResponse(endpointAddr protocol.EndpointAddr, endpointSerializedResponse []byte)
3333

34-
// GetHTTPResponse returns the user-facing HTTP response.
35-
// The received response will depend on the state of the service request context,
36-
// which is set at the time of establishing the context,
37-
// and updated using the UpdateWithResponse method above.
38-
// e.g. Calling this on a ServiceRequestContext instance which has
39-
// never been updated with a response could return an HTTP response
40-
// with a 404 HTTP status code.
34+
// GetHTTPResponse:
35+
// - Returns the user-facing HTTP response.
36+
// - Response depends on the current state of the service request context.
37+
// - State is set at context creation and updated via UpdateWithResponse.
38+
// - If never updated, may return 404 HTTP status.
4139
GetHTTPResponse() HTTPResponse
4240

43-
// GetObservations returns the set of QoS-level observations contained in the context.
44-
//
45-
// Hypothetical illustrative example.
41+
// GetObservations:
42+
// - Returns QoS-level observations in the context.
4643
//
47-
// If the context is:
48-
// - Service: Solana
49-
// - SelectedEndpoint: `endpoint_101`
50-
// - Request: `getHealth`
51-
// - Endpoint response: an error
52-
//
53-
// Then the observation can be:
54-
// - `endpoint_101` is unhealthy.
44+
// Example:
45+
// Context:
46+
// - Service: Solana
47+
// - SelectedEndpoint: `endpoint_101`
48+
// - Request: `getHealth`
49+
// - Endpoint response: error
50+
// Observation:
51+
// - `endpoint_101` is unhealthy.
5552
GetObservations() qos.Observations
5653

57-
// GetEndpointSelector is part of this interface to enable more specialized endpoint
58-
// selection, e.g. method-based endpoint selection for an EVM blockchain service request.
54+
// GetEndpointSelector:
55+
// - Enables specialized endpoint selection (e.g., method-based selection for EVM requests).
5956
GetEndpointSelector() protocol.EndpointSelector
6057
}
6158

62-
// QoSContextBuilder builds the QoS context required for handling
63-
// all steps of a service request, e.g. generating a user-facing
64-
// HTTP response from an endpoint's response.
59+
// QoSContextBuilder
60+
//
61+
// Builds the QoS context required for all steps of a service request.
62+
// Example: Generate a user-facing HTTP response from an endpoint's response.
6563
type QoSContextBuilder interface {
66-
// ParseHTTPRequest ensures that an HTTP request represents a valid request on the target service.
64+
// ParseHTTPRequest:
65+
// - Ensures the HTTP request is valid for the target service.
6766
ParseHTTPRequest(context.Context, *http.Request) (RequestQoSContext, bool)
6867

69-
// ParseWebsocketRequest ensures that a WebSocket request represents a valid request on the target service.
70-
// WebSocket connection requests do not have a body so there is no need to parse anything.
71-
// As long as the service supports WebSocket connections, this method should return a valid RequestQoSContext.
68+
// ParseWebsocketRequest:
69+
// - Ensures a WebSocket request is valid for the target service.
70+
// - WebSocket connection requests have no body, so no parsing needed.
71+
// - If service supports WebSocket, returns a valid RequestQoSContext.
7272
ParseWebsocketRequest(context.Context) (RequestQoSContext, bool)
7373
}
7474

75-
// QoSEndpointCheckGenerator returns one or more service request contexts
76-
// that can provide data on the quality of an enpoint by sending it the
77-
// corresponding payloads and parsing its response.
78-
// These checks are service-specific, i.e. the QoS instance for a
79-
// service decides what checks should be done against an endpoint.
75+
// QoSEndpointCheckGenerator
76+
//
77+
// Returns one or more service request contexts that:
78+
// - Provide data on endpoint quality by sending payloads and parsing responses.
79+
// - Checks are service-specific; the QoS instance decides what checks to run.
8080
type QoSEndpointCheckGenerator interface {
81-
// TODO_FUTURE: add a GetOptionalQualityChecks() method, e.g. to enable
82-
// a higher level of quality of service by collecting endpoints' latency
83-
// in responding to certain requests.
81+
// TODO_FUTURE:
82+
// - Add GetOptionalQualityChecks() to collect additional QoS data (e.g., endpoint latency).
8483
//
85-
// GetRequiredQualityChecks returns the set of quality checks required by
86-
// the a QoS instance to assess the validity of an endpoint.
87-
// e.g. An EVM-based blockchain service QoS may decide to skip querying an endpoint on
88-
// its current block height if it has already failed the chain ID check.
84+
// GetRequiredQualityChecks:
85+
// - Returns required quality checks for a QoS instance to assess endpoint validity.
86+
// - Example: EVM QoS may skip block height check if chain ID check already failed.
8987
GetRequiredQualityChecks(protocol.EndpointAddr) []RequestQoSContext
9088
}
9189

92-
// TODO_IMPLEMENT: Add one QoS instance per service that is to be supported by the gateway, implementing the QoSService interface below.
93-
// e.g. a QoSService implementation for Ethereum, another for Solana, and third one for a RESTful service.
90+
// TODO_IMPLEMENT:
91+
// - Add a QoS instance per service supported by the gateway (e.g., Ethereum, Solana, RESTful).
9492
//
95-
// QoSService represents the embedded definition of a service, e.g. a JSONRPC blockchain.
96-
// It is broken into several pieces to clarify its responsibilities:
97-
// 1. QoSRequestParser: Translates a service request from a supported format (currently only HTTP) into a service request context.
98-
// 2. EndpointSelector: chooses the best endpoint for performing a particular service request.
93+
// QoSService:
94+
// - Represents the embedded definition of a service (e.g., JSONRPC blockchain).
95+
// - Responsibilities:
96+
// 1. QoSRequestParser: Translates service requests (currently only HTTP) into service request contexts.
97+
// 2. EndpointSelector: Chooses the best endpoint for a specific service request.
9998
type QoSService interface {
10099
QoSContextBuilder
101100
QoSEndpointCheckGenerator
102101

103-
// ApplyObservations is used to apply QoS-related observations to the local QoS instance.
104-
// The observations can be either of:
105-
// - "local": from requests sent to an endpoint by **THIS** PATH instance
106-
// - "shared": from QoS observations shared by **OTHER** PATH instances.
102+
// ApplyObservations:
103+
// - Applies QoS-related observations to the local QoS instance.
104+
// - TODO_FUTURE: Observations can be:
105+
// - "local": from requests sent to an endpoint by THIS PATH instance.
106+
// - "shared": from QoS observations shared by OTHER PATH instances.
107107
ApplyObservations(*qos.Observations) error
108108

109-
// HydrateDisqualifiedEndpointsResponse hydrates the disqualified endpoint response with the QoS-specific data.
109+
// HydrateDisqualifiedEndpointsResponse:
110+
// - Fills the disqualified endpoint response with QoS-specific data.
110111
HydrateDisqualifiedEndpointsResponse(protocol.ServiceID, *devtools.DisqualifiedEndpointResponse)
111112
}

0 commit comments

Comments
 (0)