Skip to content

feat(instrumentation): otel collector#61

Open
ocdbytes wants to merge 13 commits into
mainfrom
aj/otel-collector
Open

feat(instrumentation): otel collector#61
ocdbytes wants to merge 13 commits into
mainfrom
aj/otel-collector

Conversation

@ocdbytes

@ocdbytes ocdbytes commented Mar 27, 2025

Copy link
Copy Markdown
Contributor
  • Added metrics for rpc, batches and mempool.
  • Updated RiscZero version.

@ocdbytes ocdbytes changed the title feat(instrumentation): Aj/otel collector feat(instrumentation): otel collector Mar 27, 2025
Comment thread core/src/state_machine.rs Outdated
old_nexus_headers: &HeaderStore,
txs: &Vec<Transaction>,
) -> Result<(Option<TreeUpdateBatch>, StateUpdate, HashMap<H256, bool>), Error> {
self.block_number_metrics.number_of_transactions_batch.record(txs.len() as u64, &[]);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we do both of this, in lib.rs? state_machine is only supposed to be for pre_execution of batch ideally. We can add this to fn save_batch_information

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Here only one metrics is recorded is it ideal to create a function in lib ? If it was repeated then it should makes sense. wdyt ?

Comment thread core/src/metrics.rs

#[derive(Clone)]
pub struct ExecutionMetrics {
pub batch_execution_time: Histogram<u64>,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we rebase to vr/sequencing, so execution metrics and prover metrics can be separated.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

echo "${HOME}/.risc0/bin" >> $GITHUB_PATH
# Need to install for specific version next.
${HOME}/.risc0/bin/rzup install
${HOME}/.risc0/bin/rzup install cargo-risczero 1.2.0

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this intentional?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes. As I updated the risc 0 version we don't need to install the cargo components separately.

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