Skip to content

op-node: Add metrics#2840

Merged
mergify[bot] merged 2 commits intodevelopfrom
feat/op-node-metrics
Jun 27, 2022
Merged

op-node: Add metrics#2840
mergify[bot] merged 2 commits intodevelopfrom
feat/op-node-metrics

Conversation

@mslipper
Copy link
Copy Markdown
Contributor

@mslipper mslipper commented Jun 23, 2022 •
edited
Loading

Adds a metrics server config and tracks metrics for the RPC server and RPC client calls. This PR is just a start - there are many more things we can track. Anything that might be affected by the batch derivation work was deliberately excluded.

Note that the wrapper for the RPC client introduces a generic, which may break older versions of goimports.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Jun 23, 2022 •
edited
Loading

⚠️ No Changeset found

Latest commit: b8ac8f7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jun 23, 2022

This PR changes implementation code, but doesn't include a changeset. Did you forget to add one?

@mslipper mslipper force-pushed the feat/op-node-metrics branch 10 times, most recently from 83753ca to 72bea45 Compare June 23, 2022 03:57
@github-actions github-actions bot added 2-reviewers C-protocol-critical Category: Modifies protocol-critical code A-cannon Area: cannon labels Jun 23, 2022
@mergify mergify bot requested a review from smartcontracts June 23, 2022 03:58
@mslipper mslipper force-pushed the feat/op-node-metrics branch from 72bea45 to 5172af4 Compare June 23, 2022 03:59
@mslipper mslipper removed the C-protocol-critical Category: Modifies protocol-critical code label Jun 23, 2022
@github-actions github-actions bot removed A-cannon Area: cannon 2-reviewers labels Jun 23, 2022
@mslipper mslipper removed the request for review from smartcontracts June 23, 2022 04:00
@mslipper mslipper force-pushed the feat/op-node-metrics branch 2 times, most recently from aeafc39 to 17f151d Compare June 23, 2022 04:04
protolambda
protolambda reviewed Jun 23, 2022
View reviewed changes
@mslipper mslipper force-pushed the feat/op-node-metrics branch from f7e8d1c to 9815242 Compare June 23, 2022 19:50
trianglesphere
trianglesphere reviewed Jun 23, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@trianglesphere trianglesphere left a comment

Choose a reason for hiding this comment

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

I'm excited for these, I've got a bunch that I want to add

@mslipper mslipper force-pushed the feat/op-node-metrics branch from 9815242 to 5e750f6 Compare June 24, 2022 03:52
@mslipper mslipper force-pushed the feat/op-node-metrics branch 3 times, most recently from 05bfb2f to 67c1c42 Compare June 24, 2022 16:03
@mslipper
op-node: Add metrics
a57c1ea 
Adds a metrics server config and tracks metrics for the RPC server and RPC client calls. This PR is just a start - there are many more things we can track. Anything that might be affected by the batch derivation work was deliberately excluded.
@mslipper mslipper force-pushed the feat/op-node-metrics branch from 67c1c42 to a57c1ea Compare June 24, 2022 16:10
protolambda
protolambda approved these changes Jun 24, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@protolambda protolambda left a comment

Choose a reason for hiding this comment

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

Looks good to me. Some nitpicks, but we can merge as-is.

The op-batcher and op-proposer already import op-node, so we can reuse the metrics tooling in those that way, but maybe we want to consider having a more general go module of go service building blocks that we want to reuse. E.g. the log configuration and CLI flags in the rollup-node are also useful to share (and that way cli flags are also more consistent).

@mslipper
Copy link
Copy Markdown
Contributor Author

mslipper commented Jun 24, 2022

maybe we want to consider having a more general go module of go service building blocks that we want to reuse

I think we should definitely do this. Something similar to what Kelvin did with BaseServiceV2 would be ideal.

tynes
tynes reviewed Jun 27, 2022
View reviewed changes
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jun 27, 2022

This PR has been added to the merge queue, and will be merged soon.

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jun 27, 2022

This PR is next in line to be merged, and will be merged as soon as checks pass.

@mergify mergify bot merged commit 33a5914 into develop Jun 27, 2022
@mergify mergify bot deleted the feat/op-node-metrics branch June 27, 2022 22:00
@mergify mergify bot removed the on-merge-train label Jun 27, 2022
theochap added a commit that referenced this pull request Dec 10, 2025
@theochap
feat(node/l1-reorg): add l1 reorg tests (#2840)
7409598 
## Description

This PR adds l1 reorg tests to CI

This PR also fixes a couple of bugs/race conditions that were causing
the derivation to halt in case of reorg.

Close #2835
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tynes tynes tynes approved these changes

@tuxcanfly tuxcanfly Awaiting requested review from tuxcanfly

+2 more reviewers

@trianglesphere trianglesphere trianglesphere approved these changes

@protolambda protolambda protolambda approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mslipper @tynes @trianglesphere @protolambda