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

Add tracing support to node#1940

Merged
coriolinus merged 37 commits intomasterfrom
prgn-tracing
Nov 20, 2020
Merged

Add tracing support to node#1940
coriolinus merged 37 commits intomasterfrom
prgn-tracing

Conversation

@coriolinus
Copy link
Contributor

@coriolinus coriolinus commented Nov 10, 2020

No description provided.

@github-actions github-actions bot added the A3-in_progress Pull request is in progress. No review needed at this stage. label Nov 10, 2020
montekki
montekki reviewed Nov 10, 2020
View reviewed changes
ordian
ordian reviewed Nov 10, 2020
View reviewed changes
@coriolinus coriolinus added 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. labels Nov 11, 2020
@coriolinus coriolinus marked this pull request as ready for review November 12, 2020 15:18
@github-actions github-actions bot added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Nov 12, 2020
@coriolinus coriolinus requested a review from rphmeier November 19, 2020 10:10
@coriolinus
Copy link
Contributor Author

coriolinus commented Nov 20, 2020

bot merge

@ghost
Copy link

ghost commented Nov 20, 2020

Missing process info; check that the PR belongs to a project column.

Merge can be attempted if:

  • The PR has approval from two core-devs (or one if the PR is labelled insubstantial).
  • The PR has approval from a member of substrateteamleads.
  • The PR is attached to a project column and has approval from the project owner.

See https://github.com/paritytech/parity-processbot#faq

@coriolinus coriolinus merged commit f7ea3d0 into master Nov 20, 2020
@coriolinus coriolinus deleted the prgn-tracing branch November 20, 2020 11:02
ordian
ordian reviewed Nov 20, 2020
View reviewed changes
node/service/src/lib.rs
info!(
block_number = %block,
delay = %delay,
"GRANDPA scheduled voting pause set for block #{} with a duration of {} blocks.",
Copy link

@ordian ordian Nov 20, 2020

Choose a reason for hiding this comment

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

sorry for being late to the party

there are a few places left where we print the data both in the message and via structured logging and it's not clear to me whether we need the former and what is the best practice here

Copy link
Contributor Author

@coriolinus coriolinus Nov 20, 2020

Choose a reason for hiding this comment

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

In cases like this where I retained both, it's because it wasn't immediately clear how to rewrite the message while preserving clarity.

ordian added a commit that referenced this pull request Nov 23, 2020
@ordian
Merge branch 'master' into ao-session-info
d50073b 
* master:
  Improve collator side of the collator-protocol (#1955)
  add parity-keyring to install instructions (#1993)
  sane messaging defaults (#1994)
  cleanup validator discovery (#1992)
  Add Prometheus timers to the subsystems (#1923)
  Add tracing support to node (#1940)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@rphmeier rphmeier Awaiting requested review from rphmeier

2 more reviewers

@ordian ordian ordian left review comments

@montekki montekki montekki approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@coriolinus @montekki @ordian