Skip to content

ENG-1670 - improve proxyd metrics, support local rate limiter#1782

Merged
mslipper merged 1 commit intoethereum-optimism:developfrom
mslipper:feat/eng-1670
Nov 24, 2021
Merged

ENG-1670 - improve proxyd metrics, support local rate limiter#1782
mslipper merged 1 commit intoethereum-optimism:developfrom
mslipper:feat/eng-1670

Conversation

@mslipper
Copy link
Contributor

@mslipper mslipper commented Nov 18, 2021

This PR improves proxyd metrics, and removes the requirement to use Redis with the rate limiter.

New metrics

rpc_backend_http_response_codes_total{"auth", "backend_name", "method_name", "status_code"} - This counter tracks HTTP error codes returned from each proxy backend. It's maintained as a separate metric from rpc_errors_total since rpc_errors_total keeps track of errors found in the JSON-RPC response body.

http_response_codes_total - This counter tracks HTTP response codes returned to the client. Usually, this will be 200 except in a couple of exceptional cases:

  • 503 when there are no backends available.
  • 400 when the request payload is inavlid.
  • 401 when a bad auth token is provided.
  • 404 when a path can't be found.

Other errors will return either a 500, or a 200 with a JSON-RPC response code. HTTP error codes other than 400s are not proxied from the upstream because they are inconsistent among different infrastructure providers.

Rate limiting

Redis is no longer required to apply rate limiting. If the [redis] stanza is not provided in the config, then proxyd will use an in-memory rate limiter instead. To remove rate limiting from an endpoint altogether, remove Redis from the config and set the RPS and concurrency limits to zero.

User agent logging

The user agent is now logged. It looks like this:

INFO [11-18|09:00:59.516] received RPC request                     req_id=eb133e436364c84a179e auth=foobar user_agent="Paw/3.3.1 (Macintosh; OS X/11.4.0) GCDHTTPRequest"
INFO [11-18|09:00:59.622] forwarded RPC request                    method=eth_chainId auth=foobar req_id=eb133e436364c84a179e

You can use the req_id to correlate subsequent log lines with the user agent.

@changeset-bot
Copy link

changeset-bot bot commented Nov 18, 2021

🦋 Changeset detected

Latest commit: da6138f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@eth-optimism/proxyd Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@tynes
Copy link
Contributor

tynes commented Nov 19, 2021

Can you add a changeset?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spacing is off here, mixed tabs + spaces

@mslipper mslipper force-pushed the feat/eng-1670 branch 2 times, most recently from 2e00f18 to ea6db40 Compare November 23, 2021 17:25
@codecov-commenter
Copy link

Codecov Report

Merging #1782 (ea6db40) into develop (d59341a) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1782   +/-   ##
========================================
  Coverage    71.81%   71.81%           
========================================
  Files           69       69           
  Lines         2303     2303           
  Branches       344      344           
========================================
  Hits          1654     1654           
  Misses         649      649           
Flag Coverage Δ
batch-submitter 61.56% <ø> (ø)
contracts 87.96% <ø> (ø)
core-utils 56.53% <ø> (ø)
data-transport-layer 38.23% <ø> (ø)
message-relayer 70.86% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ontracts/contracts/L1/verification/BondManager.sol 100.00% <0.00%> (ø)
...tracts/contracts/L1/messaging/L1StandardBridge.sol 97.05% <0.00%> (ø)
...racts/contracts/L1/rollup/StateCommitmentChain.sol 87.50% <0.00%> (ø)
...acts/contracts/L1/rollup/ChainStorageContainer.sol 70.00% <0.00%> (ø)
.../contracts/L1/messaging/L1CrossDomainMessenger.sol 96.77% <0.00%> (ø)
.../contracts/L1/rollup/CanonicalTransactionChain.sol 86.66% <0.00%> (ø)
.../contracts/libraries/bridge/CrossDomainEnabled.sol 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d59341a...ea6db40. Read the comment docs.

@mslipper mslipper merged commit 24cc244 into ethereum-optimism:develop Nov 24, 2021
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.

3 participants