Skip to content

Comments

Correctly instrument spawned tokio tasks#1

Open
keefertaylor wants to merge 7 commits intomainfrom
keefer/span-traces
Open

Correctly instrument spawned tokio tasks#1
keefertaylor wants to merge 7 commits intomainfrom
keefer/span-traces

Conversation

@keefertaylor
Copy link
Member

Description

This change contains a small fix to log output

Instrumenting a JoinHandle will not correctly instrument the task. Instead, we should instrument the task itself, which will correctly display metrics.

Drive-by changes

N/A

Related issues

N/A

Backward compatibility

N/A

Testing

Here's logging output for the MerkleTreeSyncer. Log format is set to compact in both. You'll note that in the before example, the instrumentation span is not logged to the console, while in the latter it does.

Before this change:

  2025-03-01T04:48:32.076177Z  INFO hyperlane_base::contract_sync::cursors::sequence_aware::backward: return: Ok(Some(21921811..=21923810))
    at hyperlane-base/src/contract_sync/cursors/sequence_aware/backward.rs:100
    in hyperlane_base::contract_sync::cursors::sequence_aware::backward::get_next_range with self: BackwardSequenceAwareSyncCursor { chunk_size: 1999, last_indexed_snapshot: LastIndexedSnapshot { sequence: Some(59947), at_block: 21923816 }, current_indexing_snapshot: Some(TargetSnapshot { sequence: 59946, at_block: 21923810 }), index_mode: Block, domain: HyperlaneDomain(ethereum (1)) }
    in hyperlane_base::contract_sync::fetch_logs_with_cursor with cursor: ForwardBackwardSequenceAwareSyncCursor { forward: ForwardSequenceAwareSyncCursor { chunk_size: 1999, last_indexed_snapshot: LastIndexedSnapshot { sequence: Some(61117), at_block: 21949711 }, current_indexing_snapshot: TargetSnapshot { sequence: 61118, at_block: 21949712 }, target_snapshot: None, index_mode: Block, domain: HyperlaneDomain(ethereum (1)) }, backward: BackwardSequenceAwareSyncCursor { chunk_size: 1999, last_indexed_snapshot: LastIndexedSnapshot { sequence: Some(59947), at_block: 21923816 }, current_indexing_snapshot: Some(TargetSnapshot { sequence: 59946, at_block: 21923810 }), index_mode: Block, domain: HyperlaneDomain(ethereum (1)) }, last_direction: Backward }, domain: "ethereum"
    in hyperlane_base::contract_sync::ContractSync with label: "merkle_tree_hook", domain: "ethereum"

After:

  2025-03-01T04:47:15.793466Z  INFO hyperlane_base::contract_sync::cursors::sequence_aware::backward: return: Ok(Some(21941709..=21943708))
    at hyperlane-base/src/contract_sync/cursors/sequence_aware/backward.rs:100
    in hyperlane_base::contract_sync::cursors::sequence_aware::backward::get_next_range with self: BackwardSequenceAwareSyncCursor { chunk_size: 1999, last_indexed_snapshot: LastIndexedSnapshot { sequence: Some(60846), at_block: 21943780 }, current_indexing_snapshot: Some(TargetSnapshot { sequence: 60845, at_block: 21943708 }), index_mode: Block, domain: HyperlaneDomain(ethereum (1)) }
    in hyperlane_base::contract_sync::fetch_logs_with_cursor with cursor: ForwardBackwardSequenceAwareSyncCursor { forward: ForwardSequenceAwareSyncCursor { chunk_size: 1999, last_indexed_snapshot: LastIndexedSnapshot { sequence: Some(61117), at_block: 21949705 }, current_indexing_snapshot: TargetSnapshot { sequence: 61118, at_block: 21949706 }, target_snapshot: None, index_mode: Block, domain: HyperlaneDomain(ethereum (1)) }, backward: BackwardSequenceAwareSyncCursor { chunk_size: 1999, last_indexed_snapshot: LastIndexedSnapshot { sequence: Some(60846), at_block: 21943780 }, current_indexing_snapshot: Some(TargetSnapshot { sequence: 60845, at_block: 21943708 }), index_mode: Block, domain: HyperlaneDomain(ethereum (1)) }, last_direction: Backward }, domain: "ethereum"
    in hyperlane_base::contract_sync::ContractSync with label: "merkle_tree_hook", domain: "ethereum"
    in validator::validator::MerkleTreeHookSyncer

@ameten
Copy link

ameten commented Mar 3, 2025

Thank you for the contribution! It will be great to implement the same fix in relayer and scraper so that they are consistent with this change and we will be able to merge it.

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