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

Structured logs for relayer logic #1491

Merged
merged 37 commits into from
Jan 17, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
a29c9c7
More structure in logs, pass 1
adizere Oct 25, 2021
4df293f
Pass 2
adizere Oct 25, 2021
5ae8e42
Pass 3
adizere Oct 25, 2021
50ff524
Resolving todos, refactoring
adizere Oct 25, 2021
794a7bf
Better config.toml comment
adizere Oct 26, 2021
cc5e158
Merge branch 'master' into adi/structured_logs
adizere Nov 26, 2021
d75c832
Post-merge fixes
adizere Nov 26, 2021
9d17f44
Merge branch 'master' into adi/structured_logs
mzabaluev Dec 17, 2021
a2c3311
Post-merge fix
mzabaluev Dec 17, 2021
5906035
Sketch: printing tx hashes from SendPacket events.
adizere Dec 20, 2021
74913b7
log the tx hashes in ibc_channel event SendPacket
mzabaluev Dec 21, 2021
dcf662a
Improve code to print out the tx hash
mzabaluev Dec 22, 2021
f9ca4db
Actually enter the tracing span
mzabaluev Dec 22, 2021
5de3f2b
Apply suggestions from code review
adizere Dec 23, 2021
1d15085
Comment explaining TrackedMsgs
adizere Dec 23, 2021
60e66e6
Removed use of TrackedEvents Display impl
adizere Dec 23, 2021
4d94d54
Merge branch 'master' into adi/structured_logs
mzabaluev Dec 23, 2021
d189d2c
Erase Display impl for TrackedMsgs
mzabaluev Dec 23, 2021
3166f76
Allow passing IDs without copy in TrackedMsgs
mzabaluev Dec 23, 2021
c93249b
Different tracking ids for creation flows
mzabaluev Dec 23, 2021
1bba81b
Redo displaying for OperationalData
mzabaluev Dec 23, 2021
6df6b63
Deabbreviate an info level log message
mzabaluev Dec 23, 2021
85c60fa
Improve logging of operational data
mzabaluev Dec 23, 2021
b8e9c7c
Merge branch 'master' into adi/structured_logs
mzabaluev Jan 5, 2022
d84bccd
Remove verbose wording on TrackedMsgs IDs
mzabaluev Jan 5, 2022
c5f7d87
Merge branch 'master' into adi/structured_logs
mzabaluev Jan 10, 2022
0507161
Fix typos in descriptions of RunError variants
mzabaluev Jan 10, 2022
914373c
Use a tracing span for task log messages
mzabaluev Jan 10, 2022
d1ae093
Rework tracing spans for background tasks
mzabaluev Jan 11, 2022
108d081
Merge branch 'master' into adi/structured_logs
mzabaluev Jan 11, 2022
e6b41f9
Erase Display impl on RelayPath, use spans instead
mzabaluev Jan 11, 2022
73d135e
Shorten or remove span IDs for supervisor tasks
mzabaluev Jan 12, 2022
6cb9023
Erase [rest] prefixes from log messages
mzabaluev Jan 12, 2022
c69aa81
Merge branch 'master' into adi/structured_logs
mzabaluev Jan 17, 2022
7ae8880
Simplification & consolidation w/ Mikhail
adizere Jan 17, 2022
bfec7d6
Changelog entry for #1491
mzabaluev Jan 17, 2022
45a9761
Merge branch 'master' into adi/structured_logs
mzabaluev Jan 17, 2022
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
5 changes: 3 additions & 2 deletions relayer/src/chain/cosmos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1054,7 +1054,8 @@ impl ChainEndpoint for CosmosSdkChain {
) -> Result<Vec<IbcEvent>, Error> {
crate::time!("send_messages_and_wait_commit");

let _span = span!(Level::DEBUG, "send_w", id = %tracked_msgs).entered();
let _span =
span!(Level::DEBUG, "send_tx_commit", id = %tracked_msgs.tracking_id()).entered();

let proto_msgs = tracked_msgs.messages();

Expand Down Expand Up @@ -1111,7 +1112,7 @@ impl ChainEndpoint for CosmosSdkChain {
) -> Result<Vec<Response>, Error> {
crate::time!("send_messages_and_wait_check_tx");

let span = span!(Level::DEBUG, "send", id = %tracked_msgs);
let span = span!(Level::DEBUG, "send_tx_check", id = %tracked_msgs.tracking_id());
Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, this is clearer!

let _enter = span.enter();

let proto_msgs = tracked_msgs.messages();
Expand Down
24 changes: 10 additions & 14 deletions relayer/src/chain/tx.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use core::fmt;

use prost_types::Any;

/// A wrapper over a vector of proto-encoded messages
Expand All @@ -15,33 +13,31 @@ pub struct TrackedMsgs {
}

impl TrackedMsgs {
pub fn new(msgs: Vec<Any>, tid: &str) -> Self {
pub fn new(msgs: Vec<Any>, tid: impl Into<String>) -> Self {
Self {
msgs,
tracking_id: tid.into(),
}
}

pub fn messages(&self) -> &Vec<Any> {
&self.msgs
}

pub fn new_single(msg: Any, tid: &str) -> Self {
pub fn new_single(msg: Any, tid: impl Into<String>) -> Self {
Self {
msgs: vec![msg],
tracking_id: tid.into(),
}
}

pub fn messages(&self) -> &Vec<Any> {
&self.msgs
}

pub fn tracking_id(&self) -> &str {
&self.tracking_id
}
}

impl From<TrackedMsgs> for Vec<Any> {
fn from(tm: TrackedMsgs) -> Vec<Any> {
tm.msgs
}
}

impl fmt::Display for TrackedMsgs {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}; len={}", self.tracking_id, self.msgs.len())
}
}
12 changes: 6 additions & 6 deletions relayer/src/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,7 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Channel<ChainA, ChainB> {
pub fn build_chan_open_init_and_send(&self) -> Result<IbcEvent, ChannelError> {
let dst_msgs = self.build_chan_open_init()?;

let tm = TrackedMsgs::new(dst_msgs, "create channel");
let tm = TrackedMsgs::new(dst_msgs, "create channel with ChannelOpenInit");
mzabaluev marked this conversation as resolved.
Show resolved Hide resolved

let events = self
.dst_chain()
Expand Down Expand Up @@ -870,7 +870,7 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Channel<ChainA, ChainB> {
pub fn build_chan_open_try_and_send(&self) -> Result<IbcEvent, ChannelError> {
let dst_msgs = self.build_chan_open_try()?;

let tm = TrackedMsgs::new(dst_msgs, "create channel");
let tm = TrackedMsgs::new(dst_msgs, "create channel with ChannelOpenTry");
mzabaluev marked this conversation as resolved.
Show resolved Hide resolved

let events = self
.dst_chain()
Expand Down Expand Up @@ -957,7 +957,7 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Channel<ChainA, ChainB> {
) -> Result<IbcEvent, ChannelError> {
let dst_msgs = channel.build_chan_open_ack()?;

let tm = TrackedMsgs::new(dst_msgs, "create channel");
let tm = TrackedMsgs::new(dst_msgs, "create channel with ChannelOpenAck");
mzabaluev marked this conversation as resolved.
Show resolved Hide resolved

let events = channel
.dst_chain()
Expand Down Expand Up @@ -1055,7 +1055,7 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Channel<ChainA, ChainB> {
) -> Result<IbcEvent, ChannelError> {
let dst_msgs = channel.build_chan_open_confirm()?;

let tm = TrackedMsgs::new(dst_msgs, "create channel");
let tm = TrackedMsgs::new(dst_msgs, "create channel with ChannelOpenConfirm");
mzabaluev marked this conversation as resolved.
Show resolved Hide resolved
let events = channel
.dst_chain()
.send_messages_and_wait_commit(tm)
Expand Down Expand Up @@ -1119,7 +1119,7 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Channel<ChainA, ChainB> {
pub fn build_chan_close_init_and_send(&self) -> Result<IbcEvent, ChannelError> {
let dst_msgs = self.build_chan_close_init()?;

let tm = TrackedMsgs::new(dst_msgs, "create channel");
let tm = TrackedMsgs::new(dst_msgs, "create channel with ChannelCloseInit");
mzabaluev marked this conversation as resolved.
Show resolved Hide resolved

let events = self
.dst_chain()
Expand Down Expand Up @@ -1200,7 +1200,7 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Channel<ChainA, ChainB> {
pub fn build_chan_close_confirm_and_send(&self) -> Result<IbcEvent, ChannelError> {
let dst_msgs = self.build_chan_close_confirm()?;

let tm = TrackedMsgs::new(dst_msgs, "create channel");
let tm = TrackedMsgs::new(dst_msgs, "create channel with ChannelCloseConfirm");
mzabaluev marked this conversation as resolved.
Show resolved Hide resolved

let events = self
.dst_chain()
Expand Down
12 changes: 6 additions & 6 deletions relayer/src/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -768,7 +768,7 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Connection<ChainA, ChainB> {
pub fn build_conn_init_and_send(&self) -> Result<IbcEvent, ConnectionError> {
let dst_msgs = self.build_conn_init()?;

let tm = TrackedMsgs::new(dst_msgs, "create connection");
let tm = TrackedMsgs::new(dst_msgs, "create connection with ConnectionOpenInit");
mzabaluev marked this conversation as resolved.
Show resolved Hide resolved

let events = self
.dst_chain()
Expand Down Expand Up @@ -828,7 +828,7 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Connection<ChainA, ChainB> {
.map_err(|e| ConnectionError::chain_query(self.dst_chain().id(), e))?;
let client_msgs = self.build_update_client_on_src(src_client_target_height)?;

let tm = TrackedMsgs::new(client_msgs, "create connection");
let tm = TrackedMsgs::new(client_msgs, "update client on source for ConnectionOpenTry");
Copy link
Member Author

Choose a reason for hiding this comment

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

I can't think of a shorter way to formulate this without losing context. I guess we can keep it as it is.

self.src_chain()
.send_messages_and_wait_commit(tm)
.map_err(|e| ConnectionError::submit(self.src_chain().id(), e))?;
Expand Down Expand Up @@ -899,7 +899,7 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Connection<ChainA, ChainB> {
pub fn build_conn_try_and_send(&self) -> Result<IbcEvent, ConnectionError> {
let dst_msgs = self.build_conn_try()?;

let tm = TrackedMsgs::new(dst_msgs, "create connection");
let tm = TrackedMsgs::new(dst_msgs, "create connection with ConnectionOpenTry");
mzabaluev marked this conversation as resolved.
Show resolved Hide resolved

let events = self
.dst_chain()
Expand Down Expand Up @@ -949,7 +949,7 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Connection<ChainA, ChainB> {
.map_err(|e| ConnectionError::chain_query(self.dst_chain().id(), e))?;
let client_msgs = self.build_update_client_on_src(src_client_target_height)?;

let tm = TrackedMsgs::new(client_msgs, "create connection");
let tm = TrackedMsgs::new(client_msgs, "update client on source for ConnectionOpenAck");

self.src_chain()
.send_messages_and_wait_commit(tm)
Expand Down Expand Up @@ -995,7 +995,7 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Connection<ChainA, ChainB> {
pub fn build_conn_ack_and_send(&self) -> Result<IbcEvent, ConnectionError> {
let dst_msgs = self.build_conn_ack()?;

let tm = TrackedMsgs::new(dst_msgs, "create connection");
let tm = TrackedMsgs::new(dst_msgs, "create connection with ConnectionOpenAck");
mzabaluev marked this conversation as resolved.
Show resolved Hide resolved

let events = self
.dst_chain()
Expand Down Expand Up @@ -1074,7 +1074,7 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Connection<ChainA, ChainB> {
pub fn build_conn_confirm_and_send(&self) -> Result<IbcEvent, ConnectionError> {
let dst_msgs = self.build_conn_confirm()?;

let tm = TrackedMsgs::new(dst_msgs, "create connection");
let tm = TrackedMsgs::new(dst_msgs, "create connection with ConnectionOpenConfirm");
mzabaluev marked this conversation as resolved.
Show resolved Hide resolved

let events = self
.dst_chain()
Expand Down
76 changes: 60 additions & 16 deletions relayer/src/link/operational_data.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use alloc::borrow::Cow;
use core::fmt;
use core::iter;
use std::time::Instant;
Expand Down Expand Up @@ -45,14 +46,10 @@ impl TrackedEvents {
&self.list
}

pub fn tracking_id(&self) -> &String {
pub fn tracking_id(&self) -> &str {
&self.tracking_id
}

pub fn len(&self) -> usize {
self.list.len()
}

pub fn set_height(&mut self, height: Height) {
for event in self.list.iter_mut() {
event.set_height(height);
Expand Down Expand Up @@ -99,25 +96,40 @@ pub struct OperationalData {
}

impl OperationalData {
pub fn new(proofs_height: Height, target: OperationalDataTarget, tn: &str) -> Self {
pub fn new(
proofs_height: Height,
target: OperationalDataTarget,
tracking_id: impl Into<String>,
) -> Self {
OperationalData {
proofs_height,
batch: vec![],
target,
scheduled_time: Instant::now(),
tracking_id: tn.to_string(),
tracking_id: tracking_id.into(),
}
}

pub fn push(&mut self, msg: TransitMessage) {
self.batch.push(msg)
}

pub fn events(&self) -> TrackedEvents {
let list = self.batch.iter().map(|gm| gm.event.clone()).collect();
/// Returns displayable information on the operation's data.
pub fn info(&self) -> OperationalInfo<'_> {
OperationalInfo {
tracking_id: Cow::Borrowed(&self.tracking_id),
target: self.target,
proofs_height: self.proofs_height,
batch_len: self.batch.len(),
}
}

/// Transforms `self` into the list of events accompanied with the tracking ID.
pub fn into_events(self) -> TrackedEvents {
let list = self.batch.into_iter().map(|gm| gm.event).collect();
TrackedEvents {
list,
tracking_id: self.tracking_id.clone(),
tracking_id: self.tracking_id,
}
}

Expand Down Expand Up @@ -163,7 +175,7 @@ impl OperationalData {
let tm = TrackedMsgs::new(msgs, &self.tracking_id);

info!(
"[{}] assembled batch of {} msgs",
"[{}] assembled batch of {} message(s)",
relay_path,
tm.messages().len()
);
Expand All @@ -172,15 +184,47 @@ impl OperationalData {
}
}

impl fmt::Display for OperationalData {
/// A lightweight informational data structure that can be extracted
/// out of [`OperationalData`] for e.g. logging purposes.
pub struct OperationalInfo<'a> {
tracking_id: Cow<'a, str>,
target: OperationalDataTarget,
proofs_height: Height,
batch_len: usize,
}

impl<'a> OperationalInfo<'a> {
pub fn target(&self) -> OperationalDataTarget {
self.target
}

/// Returns the length of the assembled batch of in-transit messages.
pub fn batch_len(&self) -> usize {
self.batch_len
}

pub fn into_owned(self) -> OperationalInfo<'static> {
let Self {
tracking_id,
target,
proofs_height,
batch_len,
} = self;
OperationalInfo {
tracking_id: tracking_id.into_owned().into(),
target,
proofs_height,
batch_len,
}
}
}

impl<'a> fmt::Display for OperationalInfo<'a> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(
f,
"{} ->{} @{}; len={}",
self.tracking_id,
self.target,
self.proofs_height,
self.batch.len(),
self.tracking_id, self.target, self.proofs_height, self.batch_len,
)
}
}
Loading