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

Add support for wasm runtime metrics try #2 #4483

Merged
merged 67 commits into from
Dec 16, 2021

Conversation

sandreim
Copy link
Contributor

@sandreim sandreim commented Dec 7, 2021

Fixes #4383

Companion for paritytech/substrate#10440

skip check-dependent-cumulus

Signed-off-by: Andrei Sandu <[email protected]>
Signed-off-by: Andrei Sandu <[email protected]>
Signed-off-by: Andrei Sandu <[email protected]>
Signed-off-by: Andrei Sandu <[email protected]>
Signed-off-by: Andrei Sandu <[email protected]>
Signed-off-by: Andrei Sandu <[email protected]>
@github-actions github-actions bot added the A3-in_progress Pull request is in progress. No review needed at this stage. label Dec 7, 2021
@sandreim sandreim requested a review from drahnr December 7, 2021 16:46
Copy link
Contributor

@drahnr drahnr left a comment

Choose a reason for hiding this comment

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

I like the approach with a second order function 👍

Signed-off-by: Andrei Sandu <[email protected]>
Signed-off-by: Andrei Sandu <[email protected]>
Signed-off-by: Andrei Sandu <[email protected]>
Signed-off-by: Andrei Sandu <[email protected]>
Signed-off-by: Andrei Sandu <[email protected]>
Signed-off-by: Andrei Sandu <[email protected]>
Signed-off-by: Andrei Sandu <[email protected]>
Signed-off-by: Andrei Sandu <[email protected]>
node/metrics/src/runtime.rs Outdated Show resolved Hide resolved
cli/src/cli.rs Outdated Show resolved Hide resolved
Signed-off-by: Andrei Sandu <[email protected]>
Copy link
Contributor

@drahnr drahnr left a comment

Choose a reason for hiding this comment

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

LGTM, a few small nits, but looks good overall. Encapsulating the metrics boilerplate is going to be addressed in a follow up PR.

node/metrics/src/runtime.rs Show resolved Hide resolved
where
F: FnOnce(MutexGuard<'_, HashMap<String, Counter<U64>>>) -> Result<(), PrometheusError>,
{
let _ = self.1.counters.lock().map(do_something).or_else(|error| {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: map_err is a mild improvement

node/metrics/src/tests.rs Show resolved Hide resolved
runtime/parachains/src/paras_inherent/mod.rs Show resolved Hide resolved
runtime/parachains/src/paras_inherent/mod.rs Outdated Show resolved Hide resolved
scripts/gitlab/test_linux_stable.sh Show resolved Hide resolved
Signed-off-by: Andrei Sandu <[email protected]>
Signed-off-by: Andrei Sandu <[email protected]>
runtime/parachains/src/paras_inherent/mod.rs Outdated Show resolved Hide resolved
Signed-off-by: Andrei Sandu <[email protected]>
Signed-off-by: Andrei Sandu <[email protected]>
@TriplEight TriplEight self-requested a review December 15, 2021 16:23
Signed-off-by: Andrei Sandu <[email protected]>
Signed-off-by: Andrei Sandu <[email protected]>
Signed-off-by: Andrei Sandu <[email protected]>
@ordian ordian added the D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. label Dec 15, 2021
runtime/metrics/src/std.rs Outdated Show resolved Hide resolved
runtime/metrics/src/lib.rs Outdated Show resolved Hide resolved
Signed-off-by: Andrei Sandu <[email protected]>
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.

One last nitpick

primitives/src/v1/metrics.rs Show resolved Hide resolved
@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot paritytech-processbot bot merged commit f2802d7 into master Dec 16, 2021
@paritytech-processbot paritytech-processbot bot deleted the sandreim/runtime_metrics_try2 branch December 16, 2021 11:56
drahnr pushed a commit that referenced this pull request Dec 16, 2021
* Add runtime metrics provider

Signed-off-by: Andrei Sandu <[email protected]>

* Runner changes

Signed-off-by: Andrei Sandu <[email protected]>

* Some sample metrics in paras_inherent

Signed-off-by: Andrei Sandu <[email protected]>

* update cargo toml

Signed-off-by: Andrei Sandu <[email protected]>

* fmt

Signed-off-by: Andrei Sandu <[email protected]>

* bug

Signed-off-by: Andrei Sandu <[email protected]>

* more fmt after merge

Signed-off-by: Andrei Sandu <[email protected]>

* Refactor metric prefix override

Signed-off-by: Andrei Sandu <[email protected]>

* fmt

Signed-off-by: Andrei Sandu <[email protected]>

* remove bug comment

Signed-off-by: Andrei Sandu <[email protected]>

* Add runtime metric primitives

Signed-off-by: Andrei Sandu <[email protected]>

* Impl trace event parsing

Signed-off-by: Andrei Sandu <[email protected]>

* Update metrics

Signed-off-by: Andrei Sandu <[email protected]>

* cargo lock

Signed-off-by: Andrei Sandu <[email protected]>

* fmt

Signed-off-by: Andrei Sandu <[email protected]>

* Fix target check

Signed-off-by: Andrei Sandu <[email protected]>

* Runtime metrics primitives

Signed-off-by: Andrei Sandu <[email protected]>

* Review feedback

Signed-off-by: Andrei Sandu <[email protected]>

* Runtime metrics crate

Signed-off-by: Andrei Sandu <[email protected]>

* Node side runtime metric changes

Signed-off-by: Andrei Sandu <[email protected]>

* use runtime CounterVec instead of macro

Signed-off-by: Andrei Sandu <[email protected]>

* fmt nice

Signed-off-by: Andrei Sandu <[email protected]>

* remove dead code

Signed-off-by: Andrei Sandu <[email protected]>

* base58 decoding

Signed-off-by: Andrei Sandu <[email protected]>

* base58 encoding

Signed-off-by: Andrei Sandu <[email protected]>

* fix warn

Signed-off-by: Andrei Sandu <[email protected]>

* typo

Signed-off-by: Andrei Sandu <[email protected]>

* Review feedback

Signed-off-by: Andrei Sandu <[email protected]>

* Finish label support

Signed-off-by: Andrei Sandu <[email protected]>

* fmt

Signed-off-by: Andrei Sandu <[email protected]>

* please compile

Signed-off-by: Andrei Sandu <[email protected]>

* add feature gate

Signed-off-by: Andrei Sandu <[email protected]>

* fmt

Signed-off-by: Andrei Sandu <[email protected]>

* Comment cargo toml

Signed-off-by: Andrei Sandu <[email protected]>

* Fix cargo toml description

Signed-off-by: Andrei Sandu <[email protected]>

* Update doc.

Signed-off-by: Andrei Sandu <[email protected]>

* switch to `runtime-metrics` feature

Signed-off-by: Andrei Sandu <[email protected]>

* fmt

Signed-off-by: Andrei Sandu <[email protected]>

* cargo toml

Signed-off-by: Andrei Sandu <[email protected]>

* fix tests

Signed-off-by: Andrei Sandu <[email protected]>

* fixes

Signed-off-by: Andrei Sandu <[email protected]>

* better ux

Signed-off-by: Andrei Sandu <[email protected]>

* from_utf8_unchecked is safe

Signed-off-by: Andrei Sandu <[email protected]>

* fmt

Signed-off-by: Andrei Sandu <[email protected]>

* Add Counter and refactor

Signed-off-by: Andrei Sandu <[email protected]>

* Fixes

Signed-off-by: Andrei Sandu <[email protected]>

* review fixes

Signed-off-by: Andrei Sandu <[email protected]>

* more fixes

Signed-off-by: Andrei Sandu <[email protected]>

* add integration test

Signed-off-by: Andrei Sandu <[email protected]>

* dev deps

Signed-off-by: Andrei Sandu <[email protected]>

* gitlab script update

Signed-off-by: Andrei Sandu <[email protected]>

* review fixes

Signed-off-by: Andrei Sandu <[email protected]>

* fix merge damage

Signed-off-by: Andrei Sandu <[email protected]>

* Run tests twice

Signed-off-by: Andrei Sandu <[email protected]>

* small fix

Signed-off-by: Andrei Sandu <[email protected]>

* typo

Signed-off-by: Andrei Sandu <[email protected]>

* cargo lock

Signed-off-by: Andrei Sandu <[email protected]>

* tests

Signed-off-by: Andrei Sandu <[email protected]>

* spellcheck happy ?

Signed-off-by: Andrei Sandu <[email protected]>

* more fixes

Signed-off-by: Andrei Sandu <[email protected]>

* Guard tracing init

Signed-off-by: Andrei Sandu <[email protected]>

* missing copyright

Signed-off-by: Andrei Sandu <[email protected]>

* update lockfile for substrate

Co-authored-by: parity-processbot <>
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. C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

runtime metrics: allow the runtime to provide metrics info
6 participants