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

Change TRACING_SET to static #7607

Merged
merged 4 commits into from
Nov 28, 2020
Merged

Change TRACING_SET to static #7607

merged 4 commits into from
Nov 28, 2020

Conversation

insipx
Copy link
Contributor

@insipx insipx commented Nov 26, 2020 •
edited
Loading

if TRACING_SET is const, set_global_default does not work , as each invocation of sp_io::init_tracing will try to re-initialize the global logger, creating panics in wasm compiled with with-tracing flags. const copies while static is a unique location.

@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Nov 26, 2020
@insipx insipx 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. and removed A0-please_review Pull request needs code review. labels Nov 26, 2020
bkchr
bkchr approved these changes Nov 26, 2020
View reviewed changes
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Don't we have a test for tracing in wasm?

primitives/io/src/lib.rs Outdated Show resolved Hide resolved
@insipx @bkchr
Update primitives/io/src/lib.rs
8cfb5e6 
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
@insipx
Copy link
Contributor Author

insipx commented Nov 26, 2020 •
edited
Loading

Don't we have a test for tracing in wasm?

There is this one:

substrate/client/executor/src/integration_tests/mod.rs

Line 662 in cff25fb

fn wasm_tracing_should_work(wasm_method: WasmExecutionMethod) {

Guessing it didn't catch this because it only tests one enter/exit span

@bkchr
Copy link
Member

bkchr commented Nov 26, 2020

Don't we have a test for tracing in wasm?

There is this one:

substrate/client/executor/src/integration_tests/mod.rs

Line 662 in cff25fb

fn wasm_tracing_should_work(wasm_method: WasmExecutionMethod) {

Guessing it didn't catch this because it only tests one enter/exit span

Can you please adapt it and verify that it failed before and now it works? Ty!

@insipx
Copy link
Contributor Author

insipx commented Nov 27, 2020

here's the result of the changed test:

as const:

as static:

@bkchr
Copy link
Member

bkchr commented Nov 28, 2020

Nice perfect! Now this error hopefully will not come back!

@bkchr bkchr merged commit b2d9aff into master Nov 28, 2020
@bkchr bkchr deleted the insipx/const-static-tracing branch November 28, 2020 08:40
clearloop added a commit to patractlabs/substrate that referenced this pull request Dec 1, 2020
@clearloop @TriplEight
@andresilva @tomaka @dependabot @apopiak @arkpar @bkchr @romanb @gnunicorn @drahnr @ordian @insipx @wheresaddie @mxinden @shawntabrizi @gui1117 @jolestar
Sync upstream on 2020-12-01 (#10)
4bcbc12 
* CI: build docs after test; publish docs after build (paritytech#7591)

docs time test/build success on master pub

* node-template: add aura to light block import pipeline (paritytech#7595)

added aura to block import pipeline

* Fix notifications sometimes not being sent (paritytech#7594)

* Fix notifications sometimes not being sent

* Add comment

* Bump rpassword from 4.0.5 to 5.0.0 (paritytech#7597)

Bumps [rpassword](https://github.com/conradkleinespel/rpassword) from 4.0.5 to 5.0.0.
- [Release notes](https://github.com/conradkleinespel/rpassword/releases)
- [Commits](conradkleinespel/rpassword@v4.0.5...v5.0.0)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* remove std feature flags for assert macros (paritytech#7600)

* remove std feature flags for assert macros

* re-add note about availability in no_std envs

* Add small header cache (paritytech#7516)

* Remove header query

* Header cache

* Fix potential race issue

* Simplify status query

* Inform sync explicitly about new best block (paritytech#7604)

* Inform sync explicitly about new best block

Instead of "fishing" the new best block out of the processed blocks, we
now tell sync directly that there is a new best block. It also makes
sure that we update the corresponding sync handshake to the new best
block. This is required for parachains as they first import blocks and
declare the new best block after being made aware of it by the relay chain.

* Adds test

* Make sure async stuff had time to run

* Bump directories from 2.0.2 to 3.0.1 (paritytech#7609)

Bumps [directories](https://github.com/soc/directories-rs) from 2.0.2 to 3.0.1.
- [Release notes](https://github.com/soc/directories-rs/releases)
- [Commits](https://github.com/soc/directories-rs/commits)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Remove `RpcMetrics` weirdness (paritytech#7608)

* Remove `RpcMetrics` weirdness

The metrics was returning an error when prometheus was not given. This
was a really weird setup, especially when compared to all other metrics
that just do nothing if there is no registry.

* Fix browser build

* Upgrade to libp2p-0.31. (paritytech#7606)

* Upgrade to libp2p-0.31.

* Address line width.

* Add generous incoming connection limit.

* Remove old noise configuration.

* Add Key Subcommand to node-template (paritytech#7615)

* Forward storage changes in manual seal (paritytech#7614)

This prevents nodes from executing the same block 2 times.

* chore/error: remove from str conversion and add deprecation notificat… (paritytech#7472)

* chore/error: remove from str conversion and add deprecation notifications

* fixup changes

* fix test looking for gone ::Msg variant

* another test fix

* one is duplicate, the other is not, so duplicates reported are n-1

* darn spaces

Co-authored-by: Andronik Ordian <write@reusable.software>

* remove pointless doc comments of error variants without any value

* low hanging fruits (for a tall person)

* moar error type variants

* avoid the storage modules for now

They are in need of a refactor, and the pain is rather large
removing all String error and DefaultError occurences.

* chore remove pointless error generic

* fix test for mocks, add a bunch of non_exhaustive

* max line width

* test fixes due to error changes

* fin

* error outputs... again

* undo stderr adjustments

* Update client/consensus/slots/src/lib.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* remove closure clutter

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* more error types

* introduce ApiError

* extract Mock error

* ApiError refactor

* even more error types

* the last for now

* chore unused deps

* another extraction

* reduce should panic, due to extended error messages

* error test happiness

* shift error lines by one

* doc tests

* white space

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Into -> From

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* remove pointless codec

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* avoid pointless self import

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

Co-authored-by: Bernhard Schuster <bernhard@parity.io>
Co-authored-by: Andronik Ordian <write@reusable.software>
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* network: don't force send block announcements (paritytech#7601)

* Change TRACING_SET to static (paritytech#7607)

* change TRACING_SET to static

* Update primitives/io/src/lib.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* modify test with nested spans

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* sc-network: Log outgoing notifications too (paritytech#7624)

* Log outgoing notifications too

* Update client/network/src/protocol/generic_proto/handler.rs

Co-authored-by: Max Inden <mail@max-inden.de>

Co-authored-by: Addie Wagenknecht <addie@nortd.com>
Co-authored-by: Max Inden <mail@max-inden.de>

* Bump console_log from 0.1.2 to 0.2.0 (paritytech#7623)

Bumps [console_log](https://github.com/iamcodemaker/console_log) from 0.1.2 to 0.2.0.
- [Release notes](https://github.com/iamcodemaker/console_log/releases)
- [Commits](https://github.com/iamcodemaker/console_log/commits)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* `sudo_as` should return a result (paritytech#7620)

* Fix wrong value put for pending_opening (paritytech#7633)

* Fix wrong value put for pending_opening

* Oops, didn't even try compiling it

* Rename pallet trait `Trait` to `Config` (paritytech#7599)

* rename Trait to Config

* add test asserting using Trait is still valid.

* fix ui tests

* resolve unresolved error nits of paritytech#7617 (paritytech#7631)

* handle executor should_panic test better

* Revert "reduce should panic, due to extended error messages"

This reverts commit c080594.

* remove excessive constraints

* remove duplicate documentation messages for error variants

* reduce T: constraints to the abs minimum

* whoops

* fewer bounds again

Co-authored-by: Bernhard Schuster <bernhard@parity.io>

* Fix bad state transition with DisabledPendingEnable+OpenDesiredByRemote (paritytech#7638)

* Renames of `Trait` to `Config` in README.md, weight templates and few minor ones (paritytech#7636)

* manual rename

* renamse in README.md

* fix template

* Fix CI Link Check (paritytech#7639)

* fix trigger fingers

* more

* Update frame/example-offchain-worker/README.md

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

* Fix cargo clippy warning in peerset. (paritytech#7641)

* Fix cargo clippy warning in peerset.

* Update client/peerset/src/lib.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Pierre Krieger <pierre.krieger1708@gmail.com>

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: Pierre Krieger <pierre.krieger1708@gmail.com>

Co-authored-by: Denis Pisarev <denis.pisarev@parity.io>
Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
Co-authored-by: Pierre Krieger <pierre.krieger1708@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>
Co-authored-by: Arkadiy Paronyan <arkady.paronyan@gmail.com>
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: Roman Borschel <romanb@users.noreply.github.com>
Co-authored-by: Benjamin Kampmann <ben@gnunicorn.org>
Co-authored-by: Bernhard Schuster <bernhard@ahoi.io>
Co-authored-by: Bernhard Schuster <bernhard@parity.io>
Co-authored-by: Andronik Ordian <write@reusable.software>
Co-authored-by: Andrew Plaza <aplaza@liquidthink.net>
Co-authored-by: Addie Wagenknecht <addie@nortd.com>
Co-authored-by: Max Inden <mail@max-inden.de>
Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
Co-authored-by: jolestar <jolestar@gmail.com>
darkfriend77 pushed a commit to mogwaicoin/substrate that referenced this pull request Jan 11, 2021
@insipx @bkchr @darkfriend77
Change TRACING_SET to static (paritytech#7607)
cce716e 
* change TRACING_SET to static

* Update primitives/io/src/lib.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* modify test with nested spans

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bkchr bkchr bkchr approved these changes

@mattrutherford mattrutherford mattrutherford approved these changes

@dvdplm dvdplm Awaiting requested review from dvdplm

@gnunicorn gnunicorn Awaiting requested review from gnunicorn

Assignees
No one assigned
Labels
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
@insipx @bkchr @mattrutherford