Skip to content

goclient: record a Gloas produce GET fallback as its own request - #3083

Open
iurii-ssv wants to merge 3 commits into
stagefrom
goclient/gloas-produce-method-label
Open

iurii-ssv wants to merge 3 commits into
stagefrom
goclient/gloas-produce-method-label

Conversation

@iurii-ssv

@iurii-ssv iurii-ssv commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Resolves the "Produce telemetry method label" follow-up from #2901.

Problem

GetGloasBeaconBlock POSTs produceBlockV4 and, per beacon node, falls back to the legacy GET when the node predates beacon-APIs#630 (a 404 or 405). The request metric labelled every produce POST, fallbacks included, and one sample covered both the rejected POST and the GET.

Change

  • Each request is recorded. The produce path records the rejected POST (with its 404/405) and the GET, each under its own method and duration.
  • firstClientResultWithRecorder gives each beacon node's attempt a recorder. firstClientResult is now a thin wrapper over it, so its other routes keep their method and duration. With no beacon clients, both now fail instead of returning a zero result with a nil error. That can't happen in production, since New requires an address, but it matches multiClientSubmit's guard.
  • Hand-rolled error codes. recordRequest reads the HTTP status of a hand-rolled request's error through responseStatusCode. Before, every error that wasn't go-eth2-client's was recorded with status 0. isNotFound and the produce fallback check, isMethodOrPathMissing (now beside it in errors.go), read the status through it too; neither changes behaviour.
  • Helper location. firstClientResult moves from ptc.go to gloas_http.go, next to the other hand-rolled request helpers it serves.
  • Comments. The touched doc comments each say only their own part. The produce docs no longer repeat the POST/GET fallback, and the interim hand-rolled requests (PTC, envelope, gloasHTTPClient) name their typed-calls follow-up, ePBS/Gloas: adopt go-eth2-client Gloas support (progressive SSZ types + gloas version + transport) #3014.

Metric impact

For anyone with dashboards or alerts on ssv.cl.request.duration:

  • http.response.error_status_code stops being 0 for failed hand-rolled requests and carries their real HTTP status instead. Affected routes (http.route_name):

    • through firstClientResult: GetGloasBeaconBlock, PayloadAttestationDuties, PayloadAttestationData, ProposerDutiesDependentRoot, and AttestationData on Gloas slots;
    • through multiClientSubmit: SubmitGloasBeaconBlock, SubmitExecutionPayloadEnvelope, SubmitPayloadAttestationMessages, SubmitProposerPreferences and SubmitBuilderPreferences.

    Routes served by go-eth2-client already reported real codes and are unchanged.

  • On a pre-Round Timer Fix #630 beacon node, GetGloasBeaconBlock records two requests per produce: a failed POST (404/405), then the GET. Its error count rises accordingly. A latency panel that isn't split by http.request.method mixes the fast rejected POST with the GET, which pulls the percentiles down.

No dashboards or alerts in this repo use the metric. End-to-end produce time is still logged by the proposer runner (took on 🧊 got gloas beacon block proposal).

Tests

  • The produce tests assert what gets recorded: one POST on success; the rejected POST plus the GET on a fallback; a single failed POST, with no fallback, on a wrong-fork response or a 500; and both failures when the fallback GET fails too.
  • TestGetGloasBeaconBlock_NextClientFallsBackToGET goes through a real GoClient with two fake beacon nodes. The first is down; the second predates Round Timer Fix #630 and serves the block through the GET fallback. GetGloasBeaconBlock itself had no test before.
  • TestFirstClientResult_NoClients covers the new guard, and TestResponseStatusCode covers both error types, a wrapped error, and errors without a status.
  • make lint (the pinned golangci-lint) and go test -race ./beacon/goclient/ pass.

@iurii-ssv
iurii-ssv requested review from a team as code owners October 5, 2026 11:55
@iurii-ssv iurii-ssv mentioned this pull request Oct 5, 2026
9 of 22 tasks
@greptile-apps

greptile-apps Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Refactors request recording for beacon node calls.

The PR appears safe to merge; no actionable regression was identified.

Summary

The PR records Gloas block-production POST and fallback GET requests separately, and reports HTTP status codes for hand-rolled request errors.

  • Existing single-request routes continue through the same recording behavior.
  • Tests cover successful POSTs, fallbacks, wrong-fork responses, and failed fallback GETs.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Try POST produceBlockV4] --> B[Record POST method, duration, and result]
  B -->|Success| C[Return block]
  B -->|404 or 405| D[Try legacy GET]
  D --> E[Record GET method, duration, and result]
  E --> F[Return GET result]
  B -->|Other error| G[Return POST error]
Loading

Reviews (1) · Last reviewed commit: "goclient: record a Gloas produce GET fal..."

@codecov

codecov Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.7%. Comparing base (f0e0453) to head (0a78296).

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@iurii-ssv
iurii-ssv requested a review from momosh-ssv October 5, 2026 14:47
momosh-ssv
momosh-ssv previously approved these changes Oct 9, 2026
@y0sher
y0sher dismissed momosh-ssv’s stale review October 13, 2026 13:07

The merge-base changed after approval.

Base automatically changed from stage to main October 13, 2026 13:18
GetGloasBeaconBlock recorded every produce under the POST method label,
including the GET fallback to a beacon node that predates the
produceBlockV4 POST (beacon-APIs#630), whose sample also absorbed the
rejected POST's time. The produce path now records each request it
makes: the rejected POST with its 404/405, then the GET.

- firstClientResultRecorded hands each beacon node's attempt a
  recorder. firstClientResult wraps it around its single fixed-method
  request, so its other routes are recorded as before.
- recordRequest reads the HTTP status of a hand-rolled request's error
  too (responseStatusCode, now shared with isNotFound). It used to
  record 0 for every error that was not go-eth2-client's.
- firstClientResult moves from ptc.go to gloas_http.go, beside the
  other hand-rolled request helpers it serves.
isMethodOrPathMissing reads the status through responseStatusCode, as
isNotFound does. Behaviour is unchanged: the produce POST returns only
hand-rolled errors. A new test pins that a POST the beacon node fails
(500) is returned without a GET retry.

The touched doc comments each say only their own part: the note on the
two error types moves to responseStatusCode, the produce docs stop
repeating the POST/GET fallback, and the PTC endpoints name their
typed-calls follow-up (#3014).
…to end

- firstClientResultRecorded becomes firstClientResultWithRecorder: both
  variants record, so the old suffix didn't tell them apart.
- With no beacon clients, it fails instead of returning the zero result
  with a nil error, as multiClientSubmit already does. New requires an
  address, so production can't reach this; the guard keeps the helper
  from reading an empty client list as a success.
- isMethodOrPathMissing moves to errors.go, next to isNotFound.
- TestGetGloasBeaconBlock_NextClientFallsBackToGET covers
  GetGloasBeaconBlock through a real GoClient: the first beacon node is
  down and the second serves the block through the GET fallback.
- The envelope publish and gloasHTTPClient docs name their typed-calls
  follow-up (#3014), as the PTC endpoints do.
- Drop a redundant require.Error.
@iurii-ssv
iurii-ssv force-pushed the goclient/gloas-produce-method-label branch from 6b75fc6 to 0a78296 Compare October 14, 2026 09:41
@iurii-ssv
iurii-ssv changed the base branch from main to stage October 14, 2026 09:43

This branch has not been deployed

No deployments
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 this pull request may close these issues.

2 participants