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

feat(o11y): Attach tracing context to ClientActor work items #7773

Merged
merged 22 commits into from
Oct 10, 2022

Conversation

nikurt
Copy link
Contributor

@nikurt nikurt commented Oct 5, 2022

Also refactor attaching span context: Use a extension trait to easily
wrap actix messages into a struct WithSpanContext that is
initialized with the current context.

cc: @nagisa

nikurt and others added 19 commits October 4, 2022 12:24
Use a faster way of converting `shard_id` to a string.
Crate `itoa` provides the fastest way of converting ints to strings.

Snapshot of metrics from a mainnet node. Note that some metrics are not incremented by 1 (`inc()`), but rather by a larger number using `inc_by()`.
```
1948843343 near_chunk_cache_hits # Not hot
399446179 near_chunk_cache_misses # Not hot
376782133 near_shard_cache_hits # Not hot
276622570 near_peer_manager_messages_time_count
239378614 near_applied_trie_deletions    # Optimized
206898552 near_database_op_latency_by_op_and_column_count
184492507 near_peer_message_sent_by_type_total
179740890 near_peer_message_received_total
179344494 near_peer_message_received_by_type_total
168098152 near_applied_trie_insertions    # Optimized
160684011 near_shard_cache_pop_hits # Not hot
100536077 near_client_triggers_time_count
100064991 near_requests_count_by_type_total
98868135 near_client_messages_processing_time_count
98868135 near_client_messages_count
97408226 near_peer_client_message_received_by_type_total
94021301 near_network_routed_msg_latency_count
80880283 partial_encoded_chunk_request_delay_count
78394032 near_shard_cache_gc_pop_misses
51277767 partial_encoded_chunk_response_delay_count
43248133 near_partial_encoded_chunk_request_processing_time_count
22664046 near_shard_cache_misses # Not hot
```
…work items.

Change `trace_span` to `debug_span` for messages handled by PeerActor. The reason is that this lets us compute the delay between a work item being passed between different actors.
…work items.

Change `trace_span` to `debug_span` for messages handled by PeerActor. The reason is that this lets us compute the delay between a work item being passed between different actors.
@nikurt nikurt marked this pull request as ready for review October 6, 2022 10:07
@nikurt nikurt requested a review from a team as a code owner October 6, 2022 10:07
@nikurt nikurt requested a review from mina86 October 6, 2022 10:07
@nikurt nikurt requested a review from nagisa October 6, 2022 10:08
/// This lets us trace execution across several actix Actors.
#[derive(actix::Message, Debug)]
#[rtype(result = "<T as actix::Message>::Result")]
pub struct WithSpanContext<T: actix::Message> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we ever want to make this more generally applicable in the future, we could implement the actix::Message trait manually, thus removing the T: actix::Message bound here. It is fine as-is for now, though. Maybe we will never get to the point where we need this sort of functionality elsehwere.

Copy link
Collaborator

@nagisa nagisa left a comment

Choose a reason for hiding this comment

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

LGTM

@near-bulldozer near-bulldozer bot merged commit 5634c53 into master Oct 10, 2022
@near-bulldozer near-bulldozer bot deleted the nikurt-context3 branch October 10, 2022 11:56
nikurt added a commit that referenced this pull request Nov 9, 2022
Also refactor attaching span context: Use a extension trait to easily
wrap actix messages into a struct `WithSpanContext` that is
initialized with the current context.

cc: @nagisa
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