Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

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

Merged

Conversation

alexggh
Copy link
Contributor

@alexggh alexggh commented May 31, 2023

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.

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 <[email protected]>
@alexggh alexggh requested a review from bredamatt May 31, 2023 16:00
@alexggh alexggh self-assigned this May 31, 2023
@alexggh alexggh added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. T4-parachains_engineering This PR/Issue is related to Parachains performance, stability, maintenance. labels May 31, 2023
Copy link
Contributor

@bredamatt bredamatt left a comment

Choose a reason for hiding this comment

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

See the latest comment above. Looks fine, but as also mentioned by @sandreim would be good to diff between peer and local.

Use the source to determine the tag name

Signed-off-by: Alexandru Gheorghe <[email protected]>
@@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

This could still be static if you use match / if let when creating the span.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it can be static, but it doesn't makes sense to be starting here since inner.child doesn't have this restriction.

@alexggh
Copy link
Contributor Author

alexggh commented Jun 6, 2023

bot merge

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. T4-parachains_engineering This PR/Issue is related to Parachains performance, stability, maintenance.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants