Prometheus metrics integration by murerfel · Pull Request #646 · integritee-network/worker

murerfel · 2022-02-07T07:18:32Z

Integrate the prometheus service client as described in #611

Use the warp http server to serve the metrics as a http resource (/metrics) on a configurable port
- NOTE: does NOT use https/TLS, but plain http
Add an o-call interface to update metrics from inside the enclave
Provides some default metrics (handled by the prometheus library itself)
In addition, added the following metrics:
- Free balance of the enclave account
- Sidechain block height
- TOP pool size

Merge [Sidechain] Integration of the peer sync mechanism #627 first, then rebase this PR

Closes #611

murerfel

Comments to explain the changes

murerfel · 2022-02-08T06:58:33Z

core-primitives/enclave-metrics/src/lib.rs

+#![cfg_attr(not(feature = "std"), no_std)]
+
+#[cfg(all(feature = "std", feature = "sgx"))]
+compile_error!("feature \"std\" and feature \"sgx\" cannot be enabled at the same time");
+
+#[cfg(all(not(feature = "std"), feature = "sgx"))]
+extern crate sgx_tstd as std;
+
+use codec::{Decode, Encode};
+
+#[derive(Encode, Decode)]
+pub enum EnclaveMetric {
+	SetSidechainBlockHeight(u64),
+	TopPoolSizeSet(u64),
+	TopPoolSizeIncrement,
+	TopPoolSizeDecrement,
+}


Just a small crate for the data model to hold metrics information that can be passed from the enclave -> untrusted service.

murerfel · 2022-02-08T07:01:12Z

enclave-runtime/src/global_components.rs

+pub static GLOBAL_RPC_AUTHOR_COMPONENT: ComponentContainer<EnclaveRpcAuthor> =
+	ComponentContainer::new();


Initializing this component was done in the sidechain crate until now - but since we have to pass in the o-call API now too, I decided to move it here into the enclave-runtime with the rest of all the global components.

service/src/account_funding.rs

service/src/prometheus_metrics.rs

do not use mut api reference to modify signer. create a new api for new signer.

…receiver

without the flag, the metrics server does not run

clangenb

Looks very cool. I only have one question.

service/src/account_funding.rs

clangenb · 2022-02-14T11:25:47Z

service/src/main.rs

-	api.signer = signer_orig;
-	Ok(())
-}
-


Nice, main.rs is constantly shrinking.

service/src/prometheus_metrics.rs

clangenb · 2022-02-14T11:31:47Z

service/src/prometheus_metrics.rs

+	let metrics_route = warp::path!("metrics").and_then(move || {
+		let handler_clone = metrics_handler.clone();
+		async move { handler_clone.handle_metrics().await }
+	});


I have problems understanding this code. Why do we need the nested move?

This took me 3 hours to figure out - and I still cannot say I understand it fully myself 🙈

First we create a lambda, into which we move the handler, that is the easy part to understand. But then every request needs its own instance of a request handler, and this is the only way I was able to make that work (it implements Fn and not only FnOnce, which would be the case if you didn't have the clone() like on line 52.
The second async move is required to move the handler clone into the async block, without the move it cannot be guaranteed that the handler clone lives long enough for the future to complete.

Ok, interesting. Well, to me it seems like you did understand the problematic here. 😄

haerdib

Looks good, some small comments :)

core-primitives/enclave-metrics/Cargo.toml

enclave-runtime/src/lib.rs

service/src/cli.yml

service/src/ocall_bridge/bridge_api.rs

service/src/prometheus_metrics.rs

sidechain/top-pool-rpc-author/Cargo.toml

haerdib

LGTM :)

clangenb

LGTM.

murerfel commented Feb 8, 2022

View reviewed changes

murerfel marked this pull request as ready for review February 8, 2022 07:23

murerfel requested review from clangenb and haerdib February 8, 2022 07:23

murerfel self-assigned this Feb 8, 2022

murerfel force-pushed the feature/fm-prometheus-metrics branch from dea9b22 to 6ecccd0 Compare February 9, 2022 06:32

Felix Müller added 8 commits February 9, 2022 09:50

WIP: prometheus metrics export service (http, using warp)

4c2faf0

refactor account funding in service main

05694cc

do not use mut api reference to modify signer. create a new api for new signer.

WIP: prometheus metrics handler

c9f874d

O-call structure for passing metrics from the enclave to the metrics …

db13574

…receiver

WIP: metrics from inside the enclave

c223494

make the metrics server port configurable

95dfc6c

fix clippy issues

869f139

fix cargo test

dc5d3b1

murerfel force-pushed the feature/fm-prometheus-metrics branch from 6ecccd0 to dc5d3b1 Compare February 9, 2022 09:11

murerfel mentioned this pull request Feb 14, 2022

[monitoring] watch for extrinsic failures and alert if teeracle not working properly integritee-network/teeracle#8

Closed

Felix Müller added 2 commits February 14, 2022 09:06

Add cli flag to enable metrics server (instead of always enabled)

690382f

without the flag, the metrics server does not run

fix cargo test

8026142

clangenb reviewed Feb 14, 2022

View reviewed changes

rename EnclaveWalletInfo to EnclaveAccountInfo

094dcf8

haerdib reviewed Feb 14, 2022

View reviewed changes

Felix Müller added 2 commits February 14, 2022 14:58

PR comment fixes

9b5b3be

Re-use components upon initialization

ffd15d3

murerfel requested review from clangenb and haerdib February 15, 2022 06:27

haerdib approved these changes Feb 15, 2022

View reviewed changes

clangenb approved these changes Feb 15, 2022

View reviewed changes

murerfel merged commit cbeff86 into master Feb 15, 2022

murerfel deleted the feature/fm-prometheus-metrics branch February 15, 2022 08:34

mosonyi mentioned this pull request Mar 22, 2023

[monitoring] watch for extrinsic failures and alert if teeracle not working properly #1227

Open

		pub static GLOBAL_RPC_AUTHOR_COMPONENT: ComponentContainer<EnclaveRpcAuthor> =
		ComponentContainer::new();

Conversation

murerfel commented Feb 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

murerfel left a comment

Choose a reason for hiding this comment

Uh oh!

murerfel Feb 8, 2022

Choose a reason for hiding this comment

Uh oh!

murerfel Feb 8, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

clangenb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

clangenb Feb 14, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

clangenb Feb 14, 2022

Choose a reason for hiding this comment

Uh oh!

murerfel Feb 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clangenb Feb 14, 2022

Choose a reason for hiding this comment

Uh oh!

haerdib left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

haerdib left a comment

Choose a reason for hiding this comment

Uh oh!

clangenb left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

murerfel commented Feb 7, 2022 •

edited

Loading

murerfel Feb 14, 2022 •

edited

Loading