From 8508c3ae57dbefd887aa3ec8101d809d869025fc Mon Sep 17 00:00:00 2001
From: Alexandru Gheorghe <49718502+alexggh@users.noreply.github.com>
Date: Tue, 6 Jun 2023 11:19:39 +0300
Subject: [PATCH] approval-distribution: Add approvals/assignments spans on all
 paths (#7317)

* approval-distribution: Add approvals/assignments spans on all paths

The approval and assignment logic gets called from multiple paths, so make sure
we create a tracing span on all paths to make debugging easier and be able and
correlate with the spans from approval-voting.

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>

* Tag each label with a difference tracing name

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>

* Address review feedback

Use the source to determine the tag name

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>

---------

Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
---
 polkadot/node/jaeger/src/spans.rs             | 13 +++++-
 .../network/approval-distribution/src/lib.rs  | 46 ++++++++++++-------
 2 files changed, 42 insertions(+), 17 deletions(-)

diff --git a/polkadot/node/jaeger/src/spans.rs b/polkadot/node/jaeger/src/spans.rs
index b17a7841094..be8bf9cd5dd 100644
--- a/polkadot/node/jaeger/src/spans.rs
+++ b/polkadot/node/jaeger/src/spans.rs
@@ -277,7 +277,7 @@ impl Span {
 	}
 
 	/// Derive a child span from `self`.
-	pub fn child(&self, name: &'static str) -> Self {
+	pub fn child(&self, name: &str) -> Self {
 		match self {
 			Self::Enabled(inner) => Self::Enabled(inner.child(name)),
 			Self::Disabled => Self::Disabled,
@@ -297,11 +297,22 @@ impl Span {
 		self
 	}
 
+	/// Attach a peer-id tag to the span.
 	#[inline(always)]
 	pub fn with_peer_id(self, peer: &PeerId) -> Self {
 		self.with_string_tag("peer-id", &peer.to_base58())
 	}
 
+	/// Attach a `peer-id` tag to the span when peer is present.
+	#[inline(always)]
+	pub fn with_optional_peer_id(self, peer: Option<&PeerId>) -> Self {
+		if let Some(peer) = peer {
+			self.with_peer_id(peer)
+		} else {
+			self
+		}
+	}
+
 	/// Attach a candidate hash to the span.
 	#[inline(always)]
 	pub fn with_candidate(self, candidate_hash: CandidateHash) -> Self {
diff --git a/polkadot/node/network/approval-distribution/src/lib.rs b/polkadot/node/network/approval-distribution/src/lib.rs
index 82f62502e47..0707716b33f 100644
--- a/polkadot/node/network/approval-distribution/src/lib.rs
+++ b/polkadot/node/network/approval-distribution/src/lib.rs
@@ -725,6 +725,21 @@ impl State {
 	) where
 		R: CryptoRng + Rng,
 	{
+		let _span = self
+			.spans
+			.get(&assignment.block_hash)
+			.map(|span| {
+				span.child(if source.peer_id().is_some() {
+					"peer-import-and-distribute-assignment"
+				} else {
+					"local-import-and-distribute-assignment"
+				})
+			})
+			.unwrap_or_else(|| jaeger::Span::new(&assignment.block_hash, "distribute-assignment"))
+			.with_string_tag("block-hash", format!("{:?}", assignment.block_hash))
+			.with_optional_peer_id(source.peer_id().as_ref())
+			.with_stage(jaeger::Stage::ApprovalDistribution);
+
 		let block_hash = assignment.block_hash;
 		let validator_index = assignment.validator;
 
@@ -985,6 +1000,21 @@ impl State {
 		source: MessageSource,
 		vote: IndirectSignedApprovalVote,
 	) {
+		let _span = self
+			.spans
+			.get(&vote.block_hash)
+			.map(|span| {
+				span.child(if source.peer_id().is_some() {
+					"peer-import-and-distribute-approval"
+				} else {
+					"local-import-and-distribute-approval"
+				})
+			})
+			.unwrap_or_else(|| jaeger::Span::new(&vote.block_hash, "distribute-approval"))
+			.with_string_tag("block-hash", format!("{:?}", vote.block_hash))
+			.with_optional_peer_id(source.peer_id().as_ref())
+			.with_stage(jaeger::Stage::ApprovalDistribution);
+
 		let block_hash = vote.block_hash;
 		let validator_index = vote.validator;
 		let candidate_index = vote.candidate_index;
@@ -1724,14 +1754,6 @@ impl ApprovalDistribution {
 				state.handle_new_blocks(ctx, metrics, metas, rng).await;
 			},
 			ApprovalDistributionMessage::DistributeAssignment(cert, candidate_index) => {
-				let _span = state
-					.spans
-					.get(&cert.block_hash)
-					.map(|span| span.child("import-and-distribute-assignment"))
-					.unwrap_or_else(|| jaeger::Span::new(&cert.block_hash, "distribute-assignment"))
-					.with_string_tag("block-hash", format!("{:?}", cert.block_hash))
-					.with_stage(jaeger::Stage::ApprovalDistribution);
-
 				gum::debug!(
 					target: LOG_TARGET,
 					"Distributing our assignment on candidate (block={}, index={})",
@@ -1751,14 +1773,6 @@ impl ApprovalDistribution {
 					.await;
 			},
 			ApprovalDistributionMessage::DistributeApproval(vote) => {
-				let _span = state
-					.spans
-					.get(&vote.block_hash)
-					.map(|span| span.child("import-and-distribute-approval"))
-					.unwrap_or_else(|| jaeger::Span::new(&vote.block_hash, "distribute-approval"))
-					.with_string_tag("block-hash", format!("{:?}", vote.block_hash))
-					.with_stage(jaeger::Stage::ApprovalDistribution);
-
 				gum::debug!(
 					target: LOG_TARGET,
 					"Distributing our approval vote on candidate (block={}, index={})",
-- 
GitLab