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.

Sign up for GitHub

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

feat: add tracing to worker and proxy #1014

Open
wants to merge 1 commit into
base: next
Choose a base branch
from
+479 −19
Open

feat: add tracing to worker and proxy #1014

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Changes

- Added tracing to the `miden-tx-prover` CLI (#1014).
- Added health check endpoints to the prover service (#1006).
- Implemented serialization for `AccountHeader` (#996).
- Updated Pingora crates to 0.4 and added polling time to the configuration file (#997).
Expand Down
223 changes: 223 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions bin/tx-prover/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,19 @@ tonic = { version = "0.12", default-features = false, features = ["prost", "code
getrandom = { version = "0.2", features = ["js"], optional = true }

[target.'cfg(not(all(target_arch = "wasm32", target_os = "unknown")))'.dependencies]
bytes = "1.0"
tonic = { version = "0.12", default-features = false, features = ["prost", "codegen", "transport"] }
once_cell = "1.19.0"
pingora = { version = "0.4", features = [ "lb" ] }
pingora-core = "0.4"
pingora-proxy = "0.4"
pingora-limits = "0.4"
opentelemetry = { version = "0.27", features = ["metrics", "trace"] }
opentelemetry-otlp = { version = "0.27", features = ["grpc-tonic"] }
opentelemetry_sdk = { version = "0.27", features = ["metrics", "rt-tokio"] }
opentelemetry-semantic-conventions = "0.27.0"
opentelemetry-jaeger = "0.22.0"
Comment on lines +45 to +46
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I would get rid of the patch versions.

tracing-opentelemetry = "0.28"

[dependencies]
async-trait = "0.1"
Expand Down
10 changes: 10 additions & 0 deletions bin/tx-prover/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,16 @@ The proxy service uses this health check to determine if a worker is available t

Both the worker and the proxy will use the `info` log level by default, but it can be changed by setting the `RUST_LOG` environment variable.

## Traces
Copy link
Contributor

Choose a reason for hiding this comment

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

I would maybe combine this and logging into one section (or make this a sub-section of logging?).

Also, I would add more details here. For example, it is not clear where tracing/logging info is written to. Is it stdout? Is it some logging file? Somewhere else? It is also not clear whether there is a way to not use Jaeger (or maybe use something else) to view the logs.

Basically, a bit more context about how tracing/logging works would be helpful.


The service uses the `tracing` crate for structured logging and tracing. Traces are enabled by default, and uses opentelemetry to export traces to a Jaeger instance. The traces can be visualized using the Jaeger UI, which can be used by running:

```bash
docker run -d -p4317:4317 -p16686:16686 jaegertracing/all-in-one:latest
Copy link
Contributor

Choose a reason for hiding this comment

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

This assumes that we have docker installed on the machine, right? If so, I would mention this.

Also, are there alternative ways to do this? We don't need to describe them but if there is a link to how to do it w/o Docker, I'd include it.

```

Then, you can access the Jaeger UI by opening `http://localhost:16686/` in your browser.

## Features

Description of this crate's feature:
Expand Down
12 changes: 11 additions & 1 deletion bin/tx-prover/src/api/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ use miden_tx_prover::generated::{
};
use tokio::{net::TcpListener, sync::Mutex};
use tonic::{Request, Response, Status};
use tracing::info;
use tracing::{debug, info, instrument};

use crate::utils::TRACING_TARGET_NAME;

pub struct RpcListener {
pub api_service: ApiServer<ProverRpcApi>,
Expand All @@ -30,10 +32,18 @@ pub struct ProverRpcApi {

#[async_trait::async_trait]
impl ProverApi for ProverRpcApi {
#[instrument(
target = TRACING_TARGET_NAME,
name = "prover:prove_transaction",
skip_all,
ret(level = "info"),
err
)]
async fn prove_transaction(
&self,
request: Request<ProveTransactionRequest>,
) -> Result<Response<ProveTransactionResponse>, tonic::Status> {
debug!(request = ?request, "Processing reply");
info!("Received request to prove transaction");

// Try to acquire a permit without waiting
Expand Down
Loading
Loading