Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(deps): migrate to jsonrpsee 0.22 #5894

Merged
merged 17 commits into from
Apr 5, 2024
Merged

chore(deps): migrate to jsonrpsee 0.22 #5894

merged 17 commits into from
Apr 5, 2024

Conversation

mattsse
Copy link
Collaborator

@mattsse mattsse commented Dec 29, 2023

the latest release of jsonrpsee removes a bunch of logging functionality which breaks our current rpc metrics setup...

not sure how to proceed with this yet

@Rjected
Copy link
Member

Rjected commented Jan 10, 2024

The changelog for 21.0 mentions that a middleware could be used, maybe we could use that
https://github.com/paritytech/jsonrpsee/releases/tag/v0.21.0

let rpc_middleware = RpcServiceBuilder::new().rpc_logger(1024);
let server = Server::builder().set_rpc_middleware(rpc_middleware).build("127.0.0.1:0").await?;

@DaniPopes DaniPopes changed the title wip:chore: migrate to jsonrpsee 0.21 chore(deps): migrate to jsonrpsee 0.22 Mar 6, 2024
@mattsse
Copy link
Collaborator Author

mattsse commented Apr 5, 2024

Okay finally pulled through.

There's currently one limitations wrt rpc metrics if both HTTP and ws are enabled on the same port. this will hopefully unblocked via a jsonrpsee feature

actually this can be added with a few more changes, but want to get this pr out of the way first

@mattsse mattsse marked this pull request as ready for review April 5, 2024 13:00
crates/rpc/ipc/src/server/mod.rs Outdated Show resolved Hide resolved
Comment on lines 1740 to 1745
// let metrics = modules.ipc.as_ref().map(RpcRequestMetrics::new).unwrap_or_default();
let ipc_path = self
.ipc_endpoint
.unwrap_or_else(|| Endpoint::new(DEFAULT_IPC_ENDPOINT.to_string()));
let ipc = builder.set_logger(metrics).build(ipc_path.path())?;
let ipc = builder
// .set_middleware(metrics)
Copy link
Member

Choose a reason for hiding this comment

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

follow up?

Copy link
Collaborator Author

@mattsse mattsse Apr 5, 2024

Choose a reason for hiding this comment

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

yep, this needs a few more changes, metrics for ipc are not that important

crates/rpc/rpc-builder/src/metrics.rs Outdated Show resolved Hide resolved
crates/rpc/rpc-engine-api/src/error.rs Outdated Show resolved Hide resolved
Copy link
Member

@onbjerg onbjerg 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 from my side

Copy link
Member

@DaniPopes DaniPopes left a comment

Choose a reason for hiding this comment

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

lgmt

"rustls",
"rustls-pemfile",
"rustls 0.21.10",
"rustls-pemfile 1.0.4",
Copy link
Member

Choose a reason for hiding this comment

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

deduped with #7018 on a future jsonrpsee version

@mattsse mattsse added this pull request to the merge queue Apr 5, 2024
Merged via the queue into main with commit 67cfb06 Apr 5, 2024
28 checks passed
@mattsse mattsse deleted the matt/bump-jsonrpsee21 branch April 5, 2024 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependencies Pull requests or issues that are about dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants