From 8482ed63565021924db59c571d8b57848cd9640a Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 17 Oct 2020 00:53:04 +0100 Subject: [PATCH 01/27] rust: Implement FFI interface to metrics crate --- Cargo.lock | 49 ++++++++++ Cargo.toml | 3 + src/rust/include/rust/metrics.h | 159 ++++++++++++++++++++++++++++++++ src/rust/src/metrics_ffi.rs | 64 +++++++++++++ src/rust/src/rustzcash.rs | 1 + 5 files changed, 276 insertions(+) create mode 100644 src/rust/include/rust/metrics.h create mode 100644 src/rust/src/metrics_ffi.rs diff --git a/Cargo.lock b/Cargo.lock index 420ae4de5..9debaf06a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -31,6 +31,15 @@ dependencies = [ "opaque-debug", ] +[[package]] +name = "aho-corasick" +version = "0.7.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7404febffaa47dac81aa44dba71523c9d069b1bdc50a77db41195149e17f68e5" +dependencies = [ + "memchr", +] + [[package]] name = "ansi_term" version = "0.12.1" @@ -531,6 +540,7 @@ dependencies = [ "group", "jubjub", "libc", + "metrics", "rand_core", "subtle", "tracing", @@ -566,6 +576,12 @@ version = "2.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "60302e4db3a61da70c0cb7991976248362f30319e88850c487b9b95bbf059e00" +[[package]] +name = "memchr" +version = "2.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0ee1c47aaa256ecabcaea351eae4a9b01ef39ed810004e298d2511ed284b1525" + [[package]] name = "memoffset" version = "0.5.6" @@ -575,6 +591,30 @@ dependencies = [ "autocfg", ] +[[package]] +name = "metrics" +version = "0.14.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "58e285601dcfb9f3a8f37a7093b9956a0df730b50848c8ac0117406aff06c851" +dependencies = [ + "metrics-macros", + "proc-macro-hack", +] + +[[package]] +name = "metrics-macros" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "11ac60cd4d3a869fd39d57baf0ed7f79fb677580d8d6655c4330d4f1126bc27b" +dependencies = [ + "lazy_static", + "proc-macro-hack", + "proc-macro2", + "quote", + "regex", + "syn", +] + [[package]] name = "num-bigint" version = "0.3.2" @@ -649,6 +689,12 @@ version = "0.2.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ac74c624d6b2d21f425f752262f42188365d7b8ff1aff74c82e45136510a4857" +[[package]] +name = "proc-macro-hack" +version = "0.5.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dbf0c48bc1d91375ae5c3cd81e3722dff1abcf81a30960240640d223f59fe0e5" + [[package]] name = "proc-macro2" version = "1.0.24" @@ -737,7 +783,10 @@ version = "1.4.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "957056ecddbeba1b26965114e191d2e8589ce74db242b6ea25fc4062427a5c19" dependencies = [ + "aho-corasick", + "memchr", "regex-syntax", + "thread_local", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 7286fdb4d..fc84bb68e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,6 +37,9 @@ zcash_primitives = "0.5" zcash_proofs = "0.5" ed25519-zebra = "2.0.0" +# Metrics +metrics = "0.14.2" + # Temporary workaround for https://github.com/myrrlyn/funty/issues/3 funty = "=1.1.0" diff --git a/src/rust/include/rust/metrics.h b/src/rust/include/rust/metrics.h new file mode 100644 index 000000000..ca753f071 --- /dev/null +++ b/src/rust/include/rust/metrics.h @@ -0,0 +1,159 @@ +// Copyright (c) 2020 The Zcash developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php . + +#ifndef ZCASH_RUST_INCLUDE_RUST_METRICS_H +#define ZCASH_RUST_INCLUDE_RUST_METRICS_H + +#include + +#ifdef __cplusplus +extern "C" { +#endif + +struct MetricsCallsite; +typedef struct MetricsCallsite MetricsCallsite; + +/// Creates a metrics callsite. +/// +/// You should usually call one of the helper macros such as `MetricsCounter` +/// instead of calling this directly. +/// +/// This MUST ONLY be called to assign a `static MetricsCallsite*`, and all +/// string arguments MUST be static `const char*` constants, and MUST be valid +/// UTF-8. +MetricsCallsite* metrics_callsite(const char* name); + +/// Increments a counter. +/// +/// Counters represent a single monotonic value, which means the value can only +/// be incremented, not decremented, and always starts out with an initial value +/// of zero. +void metrics_increment_counter(const MetricsCallsite* callsite, uint64_t value); + +/// Updates a gauge. +/// +/// Gauges represent a single value that can go up or down over time, and always +/// starts out with an initial value of zero. +void metrics_update_gauge(const MetricsCallsite* callsite, double value); + +/// Increments a gauge. +/// +/// Gauges represent a single value that can go up or down over time, and always +/// starts out with an initial value of zero. +void metrics_increment_gauge(const MetricsCallsite* callsite, double value); + +/// Decrements a gauge. +/// +/// Gauges represent a single value that can go up or down over time, and always +/// starts out with an initial value of zero. +void metrics_decrement_gauge(const MetricsCallsite* callsite, double value); + +/// Records a histogram. +/// +/// Histograms measure the distribution of values for a given set of +/// measurements, and start with no initial values. +void metrics_record_histogram(const MetricsCallsite* callsite, double value); + +#ifdef __cplusplus +} +#endif + +// +// Helper macros +// + +#ifdef __cplusplus +// Constructs a metrics callsite. +// +// The 'static constexpr' hack ensures that name is a compile-time constant +// with static storage duration. The output of this macro MUST be stored as a +// static MetricsCallsite*. +#define M_CALLSITE(name) ([&] { \ + static constexpr const char* _m_name = name; \ + return metrics_callsite(_m_name); \ +}()) +#else +// Constructs a metrics callsite. +// +// name MUST be a static constant, and the output of this macro MUST be stored +// as a static MetricsCallsite*. +#define M_CALLSITE(name) \ + metrics_callsite(name) +#endif + +// +// Metrics +// + +/// Increments a counter. +/// +/// Counters represent a single monotonic value, which means the value can only +/// be incremented, not decremented, and always starts out with an initial value +/// of zero. +/// +/// name MUST be a static constant, and MUST be a valid UTF-8 string. +#define MetricsCounter(name, value) \ + do { \ + static MetricsCallsite* CALLSITE = M_CALLSITE(name); \ + metrics_increment_counter(CALLSITE, value); \ + } while (0) + +/// Increments a counter by one. +/// +/// Counters represent a single monotonic value, which means the value can only +/// be incremented, not decremented, and always starts out with an initial value +/// of zero. +/// +/// name MUST be a static constant, and MUST be a valid UTF-8 string. +#define MetricsIncrementCounter(name) MetricsCounter(name, 1) + +/// Updates a gauge. +/// +/// Gauges represent a single value that can go up or down over time, and always +/// starts out with an initial value of zero. +/// +/// name MUST be a static constant, and MUST be a valid UTF-8 string. +#define MetricsGauge(name, value) \ + do { \ + static MetricsCallsite* CALLSITE = M_CALLSITE(name); \ + metrics_update_gauge(CALLSITE, value); \ + } while (0) + +/// Increments a gauge. +/// +/// Gauges represent a single value that can go up or down over time, and always +/// starts out with an initial value of zero. +/// +/// name MUST be a static constant, and MUST be a valid UTF-8 string. +#define MetricsIncrementGauge(name, value) \ + do { \ + static MetricsCallsite* CALLSITE = M_CALLSITE(name); \ + metrics_increment_gauge(CALLSITE, value); \ + } while (0) + +/// Decrements a gauge. +/// +/// Gauges represent a single value that can go up or down over time, and always +/// starts out with an initial value of zero. +/// +/// name MUST be a static constant, and MUST be a valid UTF-8 string. +#define MetricsDecrementGauge(name, value) \ + do { \ + static MetricsCallsite* CALLSITE = M_CALLSITE(name); \ + metrics_decrement_gauge(CALLSITE, value); \ + } while (0) + +/// Records a histogram. +/// +/// Histograms measure the distribution of values for a given set of +/// measurements, and start with no initial values. +/// +/// name MUST be a static constant, and MUST be a valid UTF-8 string. +#define MetricsHistogram(name, value) \ + do { \ + static MetricsCallsite* CALLSITE = M_CALLSITE(name); \ + metrics_record_histogram(CALLSITE, value); \ + } while (0) + +#endif // ZCASH_RUST_INCLUDE_RUST_METRICS_H diff --git a/src/rust/src/metrics_ffi.rs b/src/rust/src/metrics_ffi.rs new file mode 100644 index 000000000..111c72345 --- /dev/null +++ b/src/rust/src/metrics_ffi.rs @@ -0,0 +1,64 @@ +use libc::{c_char, c_double}; +use metrics::{try_recorder, GaugeValue, Key, KeyData}; +use std::ffi::CStr; + +pub struct FfiCallsite { + key_data: KeyData, +} + +#[no_mangle] +pub extern "C" fn metrics_callsite(name: *const c_char) -> *mut FfiCallsite { + let name = unsafe { CStr::from_ptr(name) }.to_str().unwrap(); + Box::into_raw(Box::new(FfiCallsite { + key_data: KeyData::from_name(name), + })) +} + +#[no_mangle] +pub extern "C" fn metrics_increment_counter(callsite: *const FfiCallsite, value: u64) { + if let Some(recorder) = try_recorder() { + let callsite = unsafe { callsite.as_ref().unwrap() }; + recorder.increment_counter(Key::Borrowed(&callsite.key_data), value); + } +} + +#[no_mangle] +pub extern "C" fn metrics_update_gauge(callsite: *const FfiCallsite, value: c_double) { + if let Some(recorder) = try_recorder() { + let callsite = unsafe { callsite.as_ref().unwrap() }; + recorder.update_gauge( + Key::Borrowed(&callsite.key_data), + GaugeValue::Absolute(value), + ); + } +} + +#[no_mangle] +pub extern "C" fn metrics_increment_gauge(callsite: *const FfiCallsite, value: c_double) { + if let Some(recorder) = try_recorder() { + let callsite = unsafe { callsite.as_ref().unwrap() }; + recorder.update_gauge( + Key::Borrowed(&callsite.key_data), + GaugeValue::Increment(value), + ); + } +} + +#[no_mangle] +pub extern "C" fn metrics_decrement_gauge(callsite: *const FfiCallsite, value: c_double) { + if let Some(recorder) = try_recorder() { + let callsite = unsafe { callsite.as_ref().unwrap() }; + recorder.update_gauge( + Key::Borrowed(&callsite.key_data), + GaugeValue::Decrement(value), + ); + } +} + +#[no_mangle] +pub extern "C" fn metrics_record_histogram(callsite: *const FfiCallsite, value: c_double) { + if let Some(recorder) = try_recorder() { + let callsite = unsafe { callsite.as_ref().unwrap() }; + recorder.record_histogram(Key::Borrowed(&callsite.key_data), value); + } +} diff --git a/src/rust/src/rustzcash.rs b/src/rust/src/rustzcash.rs index 677011a6b..9994995d2 100644 --- a/src/rust/src/rustzcash.rs +++ b/src/rust/src/rustzcash.rs @@ -63,6 +63,7 @@ use zcash_history::{Entry as MMREntry, NodeData as MMRNodeData, Tree as MMRTree} mod blake2b; mod ed25519; +mod metrics_ffi; mod tracing_ffi; #[cfg(test)] From a79ffa3b50aa60a8565f23b04da7fb7fee51dc7b Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 17 Oct 2020 00:54:04 +0100 Subject: [PATCH 02/27] rust: Add a Prometheus metrics exporter The -prometheusmetrics=host_name:port config option enables the metrics exporter. --- Cargo.lock | 444 +++++++++++++++++++++++++++++++- Cargo.toml | 1 + src/init.cpp | 15 ++ src/rust/include/rust/metrics.h | 8 + src/rust/src/metrics_ffi.rs | 27 ++ 5 files changed, 490 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9debaf06a..22d4e80a2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -61,6 +61,15 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "23b62fc65de8e4e7f52534fb52b0f3ed04746ae267519eef2a83941e8085068b" +[[package]] +name = "atomic-shim" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d20fdac7156779a1a30d970e838195558b4810dd06aa69e7c7461bdc518edf9b" +dependencies = [ + "crossbeam", +] + [[package]] name = "autocfg" version = "1.0.1" @@ -103,6 +112,12 @@ dependencies = [ "crunchy", ] +[[package]] +name = "bitflags" +version = "1.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693" + [[package]] name = "bitvec" version = "0.18.5" @@ -181,6 +196,12 @@ version = "1.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610" +[[package]] +name = "bytes" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b700ce4376041dcd0a327fd0097c41095743c4c8af8887265942faf1100bd040" + [[package]] name = "cfg-if" version = "0.1.10" @@ -236,7 +257,7 @@ dependencies = [ "cfg-if 0.1.10", "crossbeam-channel 0.4.4", "crossbeam-deque", - "crossbeam-epoch", + "crossbeam-epoch 0.8.2", "crossbeam-queue", "crossbeam-utils 0.7.2", ] @@ -267,7 +288,7 @@ version = "0.7.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9f02af974daeee82218205558e51ec8768b48cf524bd01d550abe5573a608285" dependencies = [ - "crossbeam-epoch", + "crossbeam-epoch 0.8.2", "crossbeam-utils 0.7.2", "maybe-uninit", ] @@ -283,7 +304,20 @@ dependencies = [ "crossbeam-utils 0.7.2", "lazy_static", "maybe-uninit", - "memoffset", + "memoffset 0.5.6", + "scopeguard", +] + +[[package]] +name = "crossbeam-epoch" +version = "0.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2584f639eb95fea8c798496315b297cf81b9b58b6d30ab066a75455333cf4b12" +dependencies = [ + "cfg-if 1.0.0", + "crossbeam-utils 0.8.3", + "lazy_static", + "memoffset 0.6.3", "scopeguard", ] @@ -341,6 +375,16 @@ dependencies = [ "crypto_api", ] +[[package]] +name = "ctor" +version = "0.1.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e98e2ad1a782e33928b96fc3948e7c355e5af34ba4de7670fe8bac2a3b2006d" +dependencies = [ + "quote", + "syn", +] + [[package]] name = "curve25519-dalek" version = "3.0.2" @@ -354,6 +398,16 @@ dependencies = [ "zeroize", ] +[[package]] +name = "dashmap" +version = "4.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e77a43b28d0668df09411cb0bc9a8c2adc40f9a048afe863e05fd43251e8e39c" +dependencies = [ + "cfg-if 1.0.0", + "num_cpus", +] + [[package]] name = "digest" version = "0.9.0" @@ -418,6 +472,12 @@ dependencies = [ "subtle", ] +[[package]] +name = "fnv" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" + [[package]] name = "fpe" version = "0.4.0" @@ -443,6 +503,21 @@ version = "0.1.31" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3a471a38ef8ed83cd6e40aa59c1ffe17db6855c18e3604d9c4ed8c08ebc28678" +[[package]] +name = "futures-channel" +version = "0.3.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8c2dd2df839b57db9ab69c2c9d8f3e8c81984781937fe2807dc6dcf3b2ad2939" +dependencies = [ + "futures-core", +] + +[[package]] +name = "futures-core" +version = "0.3.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "15496a72fabf0e62bdc3df11a59a3787429221dd0710ba8ef163d6f7a9112c94" + [[package]] name = "futures-cpupool" version = "0.1.8" @@ -453,6 +528,24 @@ dependencies = [ "num_cpus", ] +[[package]] +name = "futures-task" +version = "0.3.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fa189ef211c15ee602667a6fcfe1c1fd9e07d42250d2156382820fba33c9df80" + +[[package]] +name = "futures-util" +version = "0.3.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1812c7ab8aedf8d6f2701a43e1243acdbcc2b36ab26e2ad421eb99ac963d96d1" +dependencies = [ + "futures-core", + "futures-task", + "pin-project-lite", + "pin-utils", +] + [[package]] name = "generic-array" version = "0.14.4" @@ -486,6 +579,12 @@ dependencies = [ "subtle", ] +[[package]] +name = "hashbrown" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d7afe4a420e3fe79967a00898cc1f4db7c8a49a9333a29f8a4bd76a253d5cd04" + [[package]] name = "hermit-abi" version = "0.1.18" @@ -501,6 +600,88 @@ version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" +[[package]] +name = "http" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7245cd7449cc792608c3c8a9eaf69bd4eabbabf802713748fd739c98b82f0747" +dependencies = [ + "bytes", + "fnv", + "itoa", +] + +[[package]] +name = "http-body" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5dfb77c123b4e2f72a2069aeae0b4b4949cc7e966df277813fc16347e7549737" +dependencies = [ + "bytes", + "http", + "pin-project-lite", +] + +[[package]] +name = "httparse" +version = "1.3.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "615caabe2c3160b313d52ccc905335f4ed5f10881dd63dc5699d47e90be85691" + +[[package]] +name = "httpdate" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "494b4d60369511e7dea41cf646832512a94e542f68bb9c49e54518e0f468eb47" + +[[package]] +name = "hyper" +version = "0.14.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8bf09f61b52cfcf4c00de50df88ae423d6c02354e385a86341133b5338630ad1" +dependencies = [ + "bytes", + "futures-channel", + "futures-core", + "futures-util", + "http", + "http-body", + "httparse", + "httpdate", + "itoa", + "pin-project", + "socket2", + "tokio", + "tower-service", + "tracing", + "want", +] + +[[package]] +name = "indexmap" +version = "1.6.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "824845a0bf897a9042383849b02c1bc219c2383772efcd5c6f9766fa4b81aef3" +dependencies = [ + "autocfg", + "hashbrown", +] + +[[package]] +name = "instant" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "61124eeebbd69b8190558df225adf7e4caafce0d743919e5d6b19652314ec5ec" +dependencies = [ + "cfg-if 1.0.0", +] + +[[package]] +name = "itoa" +version = "0.4.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dd25036021b0de88a0aff6b850051563c6516d0bf53f8638938edbb9de732736" + [[package]] name = "jubjub" version = "0.5.1" @@ -541,6 +722,7 @@ dependencies = [ "jubjub", "libc", "metrics", + "metrics-exporter-prometheus", "rand_core", "subtle", "tracing", @@ -552,6 +734,15 @@ dependencies = [ "zcash_proofs", ] +[[package]] +name = "lock_api" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dd96ffd135b2fd7b973ac026d28085defbe8983df057ced3eb4f2130b0831312" +dependencies = [ + "scopeguard", +] + [[package]] name = "log" version = "0.4.14" @@ -561,6 +752,15 @@ dependencies = [ "cfg-if 1.0.0", ] +[[package]] +name = "mach" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b823e83b2affd8f40a9ee8c29dbc56404c1e34cd2710921f2801e2cf29527afa" +dependencies = [ + "libc", +] + [[package]] name = "matchers" version = "0.0.1" @@ -591,6 +791,15 @@ dependencies = [ "autocfg", ] +[[package]] +name = "memoffset" +version = "0.6.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f83fb6581e8ed1f85fd45c116db8405483899489e38406156c25eb743554361d" +dependencies = [ + "autocfg", +] + [[package]] name = "metrics" version = "0.14.2" @@ -601,6 +810,21 @@ dependencies = [ "proc-macro-hack", ] +[[package]] +name = "metrics-exporter-prometheus" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8d23bb354bd7dd5d244f2e9a389a0c3249bb6b994aca71bd6c2f7cd6fa2657fc" +dependencies = [ + "hyper", + "metrics", + "metrics-util", + "parking_lot", + "quanta", + "thiserror", + "tokio", +] + [[package]] name = "metrics-macros" version = "0.2.0" @@ -615,6 +839,56 @@ dependencies = [ "syn", ] +[[package]] +name = "metrics-util" +version = "0.6.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8ace44e2c64f785c3c37605ecf7504e5fc4efbb2936a49cc56c1084d79657a4d" +dependencies = [ + "aho-corasick", + "atomic-shim", + "crossbeam-epoch 0.9.3", + "crossbeam-utils 0.8.3", + "dashmap", + "indexmap", + "metrics", + "ordered-float", + "parking_lot", + "quanta", + "sketches-ddsketch", +] + +[[package]] +name = "mio" +version = "0.7.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf80d3e903b34e0bd7282b218398aec54e082c840d9baf8339e0080a0c542956" +dependencies = [ + "libc", + "log", + "miow", + "ntapi", + "winapi", +] + +[[package]] +name = "miow" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b9f1c5b025cda876f66ef43a113f91ebc9f4ccef34843000e0adf6ebbab84e21" +dependencies = [ + "winapi", +] + +[[package]] +name = "ntapi" +version = "0.3.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f6bb902e437b6d86e03cce10a7e2af662292c5dfef23b65899ea3ac9354ad44" +dependencies = [ + "winapi", +] + [[package]] name = "num-bigint" version = "0.3.2" @@ -667,6 +941,15 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "624a8340c38c1b80fd549087862da4ba43e08858af025b236e509b6649fc13d5" +[[package]] +name = "ordered-float" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "766f840da25490628d8e63e529cd21c014f6600c6b8517add12a6fa6167a6218" +dependencies = [ + "num-traits", +] + [[package]] name = "pairing" version = "0.18.0" @@ -677,12 +960,63 @@ dependencies = [ "group", ] +[[package]] +name = "parking_lot" +version = "0.11.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6d7744ac029df22dca6284efe4e898991d28e3085c706c972bcd7da4a27a15eb" +dependencies = [ + "instant", + "lock_api", + "parking_lot_core", +] + +[[package]] +name = "parking_lot_core" +version = "0.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fa7a782938e745763fe6907fc6ba86946d72f49fe7e21de074e08128a99fb018" +dependencies = [ + "cfg-if 1.0.0", + "instant", + "libc", + "redox_syscall 0.2.5", + "smallvec", + "winapi", +] + +[[package]] +name = "pin-project" +version = "1.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc174859768806e91ae575187ada95c91a29e96a98dc5d2cd9a1fed039501ba6" +dependencies = [ + "pin-project-internal", +] + +[[package]] +name = "pin-project-internal" +version = "1.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a490329918e856ed1b083f244e3bfe2d8c4f336407e4ea9e1a9f479ff09049e5" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "pin-project-lite" version = "0.2.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dc0e1f259c92177c30a4c9d177246edd0a3568b25756a977d0632cf8fa37e905" +[[package]] +name = "pin-utils" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" + [[package]] name = "ppv-lite86" version = "0.2.10" @@ -704,6 +1038,21 @@ dependencies = [ "unicode-xid", ] +[[package]] +name = "quanta" +version = "0.7.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e76a3afdefd0ce2c0363bf3146271e947782240ea617885dd64e56c4de9fb3c9" +dependencies = [ + "atomic-shim", + "ctor", + "libc", + "mach", + "once_cell", + "raw-cpuid", + "winapi", +] + [[package]] name = "quote" version = "1.0.9" @@ -760,12 +1109,30 @@ dependencies = [ "rand_core", ] +[[package]] +name = "raw-cpuid" +version = "9.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c27cb5785b85bd05d4eb171556c9a1a514552e26123aeae6bb7d811353148026" +dependencies = [ + "bitflags", +] + [[package]] name = "redox_syscall" version = "0.1.57" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "41cc0f7e4d5d4544e8861606a285bb08d3e70712ccc7d2b84d7c0ccfaf4b05ce" +[[package]] +name = "redox_syscall" +version = "0.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94341e4e44e24f6b591b59e47a8a027df12e008d73fd5672dbea9cc22f4507d9" +dependencies = [ + "bitflags", +] + [[package]] name = "redox_users" version = "0.3.5" @@ -773,7 +1140,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "de0737333e7a9502c789a36d7c7fa6092a49895d4faa31ca5df163857ded2e9d" dependencies = [ "getrandom", - "redox_syscall", + "redox_syscall 0.1.57", "rust-argon2", ] @@ -786,7 +1153,6 @@ dependencies = [ "aho-corasick", "memchr", "regex-syntax", - "thread_local", ] [[package]] @@ -865,6 +1231,28 @@ dependencies = [ "lazy_static", ] +[[package]] +name = "sketches-ddsketch" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "76a77a8fd93886010f05e7ea0720e569d6d16c65329dbe3ec033bbbccccb017b" + +[[package]] +name = "smallvec" +version = "1.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fe0f37c9e8f3c5a4a66ad655a93c74daac4ad00c441533bf5c6e7990bb42604e" + +[[package]] +name = "socket2" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9e3dfc207c526015c632472a77be09cf1b6e46866581aecae5cc38fb4235dea2" +dependencies = [ + "libc", + "winapi", +] + [[package]] name = "subtle" version = "2.4.0" @@ -922,6 +1310,36 @@ dependencies = [ "winapi", ] +[[package]] +name = "tokio" +version = "1.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "134af885d758d645f0f0505c9a8b3f9bf8a348fd822e112ab5248138348f1722" +dependencies = [ + "autocfg", + "libc", + "mio", + "pin-project-lite", + "tokio-macros", +] + +[[package]] +name = "tokio-macros" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "caf7b11a536f46a809a8a9f0bb4237020f70ecbf115b842360afb127ea2fda57" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "tower-service" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "360dfd1d6d30e05fda32ace2c8c70e9c0a9da713275777f5a4dbb8a1893930c6" + [[package]] name = "tracing" version = "0.1.25" @@ -982,6 +1400,12 @@ dependencies = [ "tracing-core", ] +[[package]] +name = "try-lock" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "59547bce71d9c38b83d9c0e92b6066c4253371f15005def0c30d9657f50c7642" + [[package]] name = "typenum" version = "1.13.0" @@ -1000,6 +1424,16 @@ version = "0.9.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5fecdca9a5291cc2b8dcf7dc02453fee791a280f3743cb0905f8822ae463b3fe" +[[package]] +name = "want" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1ce8a968cb1cd110d136ff8b819a556d6fb6d919363c61534f6860c7eb172ba0" +dependencies = [ + "log", + "try-lock", +] + [[package]] name = "wasi" version = "0.9.0+wasi-snapshot-preview1" diff --git a/Cargo.toml b/Cargo.toml index fc84bb68e..e4e2cfd69 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,6 +39,7 @@ ed25519-zebra = "2.0.0" # Metrics metrics = "0.14.2" +metrics-exporter-prometheus = "0.3" # Temporary workaround for https://github.com/myrrlyn/funty/issues/3 funty = "=1.1.0" diff --git a/src/init.cpp b/src/init.cpp index eef322d55..df6818dfa 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -64,6 +64,8 @@ #include "zmq/zmqnotificationinterface.h" #endif +#include + #include "librustzcash.h" using namespace std; @@ -351,6 +353,8 @@ std::string HelpMessage(HelpMessageMode mode) #ifndef WIN32 strUsage += HelpMessageOpt("-pid=", strprintf(_("Specify pid file (default: %s)"), BITCOIN_PID_FILENAME)); #endif + strUsage += HelpMessageOpt("-prometheusmetrics=:", _("Expose node metrics in the Prometheus exposition format. " + "An HTTP listener will be started on the configured hostname and port, which responds to GET requests on any request path.")); strUsage += HelpMessageOpt("-prune=", strprintf(_("Reduce storage requirements by pruning (deleting) old blocks. This mode disables wallet support and is incompatible with -txindex. " "Warning: Reverting this setting requires re-downloading the entire blockchain. " "(default: 0 = disable pruning blocks, >%u = target size in MiB to use for block files)"), MIN_DISK_SPACE_FOR_BLOCK_FILES / 1024 / 1024)); @@ -1221,6 +1225,17 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) // Count uptime MarkStartTime(); + std::string prometheusMetricsArg = GetArg("-prometheusmetrics", ""); + if (prometheusMetricsArg != "") { + // Start up the metrics runtime. This spins off a Rust thread that runs + // the Prometheus exporter. We just let this thread die at process end + // ¯\_(ツ)_/¯ + LogPrintf("metrics thread start"); + if (!metrics_run(prometheusMetricsArg.c_str())) { + return InitError(strprintf(_("Failed to start Prometheus metrics exporter on '%s'"), prometheusMetricsArg)); + } + } + if ((chainparams.NetworkIDString() != "regtest") && GetBoolArg("-showmetrics", isatty(STDOUT_FILENO)) && !fPrintToConsole && !GetBoolArg("-daemon", false)) { diff --git a/src/rust/include/rust/metrics.h b/src/rust/include/rust/metrics.h index ca753f071..13aa23bb6 100644 --- a/src/rust/include/rust/metrics.h +++ b/src/rust/include/rust/metrics.h @@ -11,6 +11,14 @@ extern "C" { #endif +/// Initializes the metrics runtime and runs the Prometheus exporter in a new +/// thread. +/// +/// listen_address is a string like :. +/// +/// Returns false if listen_address was not a valid SocketAddr. +bool metrics_run(const char* listen_address); + struct MetricsCallsite; typedef struct MetricsCallsite MetricsCallsite; diff --git a/src/rust/src/metrics_ffi.rs b/src/rust/src/metrics_ffi.rs index 111c72345..eecdcdfc2 100644 --- a/src/rust/src/metrics_ffi.rs +++ b/src/rust/src/metrics_ffi.rs @@ -1,6 +1,33 @@ use libc::{c_char, c_double}; use metrics::{try_recorder, GaugeValue, Key, KeyData}; +use metrics_exporter_prometheus::PrometheusBuilder; use std::ffi::CStr; +use std::net::SocketAddr; +use tracing::error; + +#[no_mangle] +pub extern "C" fn metrics_run(listen_address: *const c_char) -> bool { + let listen_address = unsafe { CStr::from_ptr(listen_address) }.to_str().unwrap(); + listen_address + .parse::() + .map_err(|e| { + error!( + "Invalid Prometheus metrics address '{}': {}", + listen_address, e + ); + () + }) + .and_then(|addr| { + PrometheusBuilder::new() + .listen_address(addr) + .install() + .map_err(|e| { + error!("Failed to start Prometheus metrics exporter: {:?}", e); + () + }) + }) + .is_ok() +} pub struct FfiCallsite { key_data: KeyData, From 90f4d4830724ffda1813b17c334be39262848028 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sat, 17 Oct 2020 02:07:55 +0100 Subject: [PATCH 03/27] Add some metrics that match existing zebrad metrics The metric names match those used in zebrad, so the same visualisers can be used for both nodes. See https://github.com/ZcashFoundation/zebra/issues/1381 for discussion about potential naming changes. --- src/main.cpp | 6 ++++++ src/net.cpp | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/src/main.cpp b/src/main.cpp index cfa7dbb57..9a21065de 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -46,6 +46,7 @@ #include #include +#include using namespace std; @@ -3237,6 +3238,7 @@ void static UpdateTip(CBlockIndex *pindexNew, const CChainParams& chainParams) { "date", date.c_str(), "progress", progress.c_str(), "cache", cache.c_str()); + MetricsGauge("block.verified.block.height", pindexNew->nHeight); cvBlockChange.notify_all(); } @@ -3369,6 +3371,7 @@ bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, int64_t nTime6 = GetTimeMicros(); nTimePostConnect += nTime6 - nTime5; nTimeTotal += nTime6 - nTime1; LogPrint("bench", " - Connect postprocess: %.2fms [%.2fs]\n", (nTime6 - nTime5) * 0.001, nTimePostConnect * 0.000001); LogPrint("bench", "- Connect block: %.2fms [%.2fs]\n", (nTime6 - nTime1) * 0.001, nTimeTotal * 0.000001); + MetricsIncrementCounter("block.verified.block.count"); return true; } @@ -6226,6 +6229,7 @@ bool static ProcessMessage(const CChainParams& chainparams, CNode* pfrom, string CInv inv(MSG_BLOCK, block.GetHash()); LogPrint("net", "received block %s peer=%d\n", inv.hash.ToString(), pfrom->id); + MetricsIncrementCounter("sync.downloaded.block.count"); pfrom->AddInventoryKnown(inv); @@ -6244,6 +6248,8 @@ bool static ProcessMessage(const CChainParams& chainparams, CNode* pfrom, string LOCK(cs_main); Misbehaving(pfrom->GetId(), nDoS); } + } else if (state.IsValid()) { + MetricsIncrementCounter("sync.verified.block.count"); } } diff --git a/src/net.cpp b/src/net.cpp index 322219f54..c2a7b08bd 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -31,6 +31,8 @@ #include +#include + // Dump addresses to peers.dat and banlist.dat every 15 minutes (900s) #define DUMP_ADDRESSES_INTERVAL 900 @@ -702,6 +704,7 @@ bool CNode::ReceiveMsgBytes(const char *pch, unsigned int nBytes) if (msg.complete()) { msg.nTime = GetTimeMicros(); + MetricsIncrementCounter("peer.inbound.messages"); messageHandlerCondition.notify_one(); } } @@ -1104,6 +1107,7 @@ void ThreadSocketHandler() } if (vNodesSize != nPrevNodeCount) { nPrevNodeCount = vNodesSize; + MetricsGauge("pool.num_peers", nPrevNodeCount); uiInterface.NotifyNumConnectionsChanged(nPrevNodeCount); } @@ -2001,12 +2005,14 @@ void CNode::RecordBytesRecv(uint64_t bytes) { LOCK(cs_totalBytesRecv); nTotalBytesRecv += bytes; + MetricsCounter("bytes.read", bytes); } void CNode::RecordBytesSent(uint64_t bytes) { LOCK(cs_totalBytesSent); nTotalBytesSent += bytes; + MetricsCounter("bytes.written", bytes); uint64_t now = GetTime(); if (nMaxOutboundCycleStartTime + nMaxOutboundTimeframe < now) @@ -2280,6 +2286,7 @@ void CNode::AbortMessage() UNLOCK_FUNCTION(cs_vSend) void CNode::EndMessage() UNLOCK_FUNCTION(cs_vSend) { + MetricsIncrementCounter("peer.outbound.messages"); // The -*messagestest options are intentionally not documented in the help message, // since they are only used during development to debug the networking code and are // not intended for end-users. From e5a5bc5b83b0741bdbd13d159426783d451adaac Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 17 Dec 2020 21:37:56 +0000 Subject: [PATCH 04/27] metrics: Add documentation and example configs --- contrib/metrics/README.md | 76 +++++++++++++++++++++++++++++++++ contrib/metrics/prometheus.yaml | 6 +++ 2 files changed, 82 insertions(+) create mode 100644 contrib/metrics/README.md create mode 100644 contrib/metrics/prometheus.yaml diff --git a/contrib/metrics/README.md b/contrib/metrics/README.md new file mode 100644 index 000000000..6135ce544 --- /dev/null +++ b/contrib/metrics/README.md @@ -0,0 +1,76 @@ +# zcashd metrics + +## Metrics UI + +This is the user interface that `zcashd` displays by default when run. It +displays a small selection of interesting metrics, but is not intended for +programmatic consumption. + +## RPC methods + +`zcashd` provides the following JSON-RPC methods that expose node metrics: + +- Chain: + - `getblockchaininfo`: Various state info regarding block chain processing. + - `gettxoutsetinfo`: Statistics about the unspent transparent transaction output set. + - `getmempoolinfo`: Details on the active state of the TX memory pool. +- P2P network: + - `getnetworkinfo`: Various state info regarding P2P networking. + - `getpeerinfo`: Data about each connected network node. + - `getdeprecationinfo`: The current node version and deprecation block height. +- Miscellaneous + - `getmemoryinfo`: Information about memory usage. + - `getmininginfo`: Mining-related information. + - `getinfo` (deprecated): A small subset of the above metrics. + +You can see what each method provides with `zcash-cli help METHOD_NAME`. + +## Prometheus support + +`zcashd` can optionally expose an HTTP server that acts as a Prometheus scrape +endpoint. The server will respond to `GET` requests on any request path. + +To enable the endpoint, add `-prometheusmetrics=:` to your +`zcashd` configuration (either in `zcash.conf` or on the command line). After +restarting `zcashd` you can then test the endpoint by querying it: + +``` +$ curl http://: +# TYPE peer_outbound_messages counter +peer_outbound_messages 181 + +# TYPE bytes_read counter +bytes_read 3701998 + +# TYPE peer_inbound_messages counter +peer_inbound_messages 184 + +# TYPE zcashd_build_info counter +zcashd_build_info{version="v4.2.0"} 1 + +# TYPE block_verified_block_count counter +block_verified_block_count 162 +... +``` + +### Example metrics collection with Docker + +The example instructions below were tested on Windows 10 using Docker Desktop +with the WSL 2 backend: + +``` +# Create a storage volume for Grafana (once) +docker volume create grafana-storage + +# Create a storage volume for Prometheus (once) +docker volume create prometheus-storage + +# Run Prometheus +# You will need to modify ~/contrib/metrics/prometheus.yaml to match the +# endpoint configured with -prometheusmetrics (and possibly also for your Docker +# network setup). +docker run --detach -p 9090:9090 --volume prometheus-storage:/prometheus --volume ~/contrib/metrics/prometheus.yaml:/etc/prometheus/prometheus.yml prom/prometheus + +# Run Grafana +docker run --detach -p 3030:3030 --env GF_SERVER_HTTP_PORT=3030 --volume grafana-storage:/var/lib/grafana grafana/grafana +``` diff --git a/contrib/metrics/prometheus.yaml b/contrib/metrics/prometheus.yaml new file mode 100644 index 000000000..77d08e5b4 --- /dev/null +++ b/contrib/metrics/prometheus.yaml @@ -0,0 +1,6 @@ +scrape_configs: + - job_name: 'zcashd' + scrape_interval: 500ms + metrics_path: '/' + static_configs: + - targets: ['127.0.0.1:9969'] From 37b42d8a4139e3403870b4c12369dea3a2d79bfd Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 24 Dec 2020 15:01:49 +0000 Subject: [PATCH 05/27] tracing: Merge TracingSpanFields macro into TracingSpan Leverages the VA_OPT macro library, which is a polyfill for __VA_OPT__ on non-C++20 platforms, to enable TracingSpan to support optional fields. Source: https://github.com/willwray/VA_OPT License: Boost Software License, Version 1.0 --- contrib/debian/copyright | 4 + src/net.cpp | 4 +- src/rust/include/rust/VA_OPT.hpp | 152 +++++++++++++++++++++++++++++++ src/rust/include/tracing.h | 39 +++----- 4 files changed, 170 insertions(+), 29 deletions(-) create mode 100644 src/rust/include/rust/VA_OPT.hpp diff --git a/contrib/debian/copyright b/contrib/debian/copyright index 4fd3b17f9..8ec680585 100644 --- a/contrib/debian/copyright +++ b/contrib/debian/copyright @@ -124,6 +124,10 @@ Files: src/crypto/ctaes/* Copyright: Copyright (c) 2016 Pieter Wuille License: Expat +Files: src/rust/include/rust/VA_OPT.hpp +Copyright: Copyright (c) 2019 Will Wray +License: Boost-Software-License-1.0 + Files: src/rust/include/tracing.h src/rust/src/tracing_ffi.rs Copyright: Copyright (c) 2020 Jack Grigg diff --git a/src/net.cpp b/src/net.cpp index c2a7b08bd..f7d54aed6 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2224,11 +2224,11 @@ CNode::~CNode() void CNode::ReloadTracingSpan() { if (fLogIPs) { - span = TracingSpanFields("info", "net", "Peer", + span = TracingSpan("info", "net", "Peer", "id", idStr.c_str(), "addr", addrName.c_str()); } else { - span = TracingSpanFields("info", "net", "Peer", + span = TracingSpan("info", "net", "Peer", "id", idStr.c_str()); } } diff --git a/src/rust/include/rust/VA_OPT.hpp b/src/rust/include/rust/VA_OPT.hpp new file mode 100644 index 000000000..430282575 --- /dev/null +++ b/src/rust/include/rust/VA_OPT.hpp @@ -0,0 +1,152 @@ +// Copyright (c) 2019 Will Wray https://keybase.io/willwray +// +// Distributed under the Boost Software License, Version 1.0. +// http://www.boost.org/LICENSE_1_0.txt +// +// Repo: https://github.com/willwray/VA_OPT + +#pragma once + +/* + VA_OPT.hpp + ========== + + Preprocessor utilities for testing emptiness of macro arguments + and for conditional expansion based on the emptiness of ARGS. + + VA_OPT_SUPPORT(?) + 1 if __VA_OPT__ support is detected else 0 (see platform note). + + C++20's __VA_OPT__ finally provides a reliable test for empty ARGS. + The following macros use __VA_OPT__, if detected, otherwise they + provide 'polyfill' fallbacks (though with some failing edge cases). + + IS_EMPTY(...) + 1 if the ... ARGS is empty else 0. + + IFN(...) + If ... ARGS are not empty then a trailing 'paren expression' (X) + is deparenthesized to X else the trailing (X) term is consumed. + E.g. IFN()(N) vanishes, while IFN(NotEmpty)(N) -> N + + In other words, IFN(ARGS) is like __VA_OPT__, but with explicit + (ARGS) in place of an implicit __VA_ARGS__ check. + + IFE(...) + If ... ARGS is empty expand trailing 'paren expression' (X) to X + else if ARGS are not empty consume the trailing paren expression. + E.g. IFE(NotEmpty)(E) vanishes, while IFE()(E) -> E + + IFNE(...)(N,E...) + If ... ARGS are not empty expands to N else expands to E... + E.g. IFNE(ARG)(X,()) is equivalent to IFN(ARG)(X)IFE(ARG)(()) + both put back a terminating () removed by the outer macro call. + + Without VA_OPT_SUPPORT these 'emptiness' macros are not perfect; + IS_EMPTY, IFN, IFE, IFNE may cause a compile error ('too few args') + if the argument is a function-like macro name that expects ARG(s). + + IBP(...) + IS_BEGIN_PARENS macro to test if an argument is parenthesised: + 1 if ... ARGS begins with a 'paren expression' else 0. + + Platform note: Current Sept 2019 __VA_OPT__ support: + ------------- + Clang -std=c++2a enables it. GCC has it enabled without -std=c++2a + but warns "__VA_OPT__ is not available until C++2a" if another -std + flag is supplied along with -pedantic (dont know how to supress it). + MSVC TBD + + Credits + ------- + Props to pre-pro pioneers, particularly Paul Mensonides. + The 'emptiness' methods are adapted from BOOST_VMD_IS_EMPTY which, +. in turn, depends on BOOST Preprocessor's BOOST_PP_IS_BEGIN_PARENS + (adapted and exposed here as IBP 'Is Begin Parens'): + www.boost.org/doc/libs/1_71_0/libs/vmd + www.boost.org/doc/libs/1_71_0/libs/preprocessor +*/ + +#define VA_ARG1(A0,A1,...) A1 +// VA_EMPTY works only if __VA_OPT__ is supported, else always -> 1 +#define VA_EMPTY(...) VA_ARG1(__VA_OPT__(,)0,1,) + +// VA_OPT_SUPPORT helper macro for __VA_OPT__ feature detection. +// Adapted from https://stackoverflow.com/a/48045656/7443483 +// Use as #if VA_OPT_SUPPORT(?) +#define VA_OPT_SUPPORT ! VA_EMPTY + +#if VA_OPT_SUPPORT(?) + +# define IS_EMPTY(...) VA_EMPTY(__VA_ARGS__) +# define IFN(...) VA_EAT __VA_OPT__(()VA_IDENT) +# define IFE(...) VA_IDENT __VA_OPT__(()VA_EAT) +# define IFNE(...) VA_ARGTAIL __VA_OPT__((,)VA_ARG0) + +#else + +# define IS_EMPTY(...) IFP(IBP(__VA_ARGS__))(IE_GEN_0,IE_IBP)(__VA_ARGS__) +# define IFN(...) IFP(IBP(__VA_ARGS__))(GEN_IDENT,EAT_OR_IDENT)(__VA_ARGS__) +# define IFE(...) IFP(IBP(__VA_ARGS__))(GEN_EAT,IDENT_OR_EAT)(__VA_ARGS__) +# define IFNE(...) IFP(IBP(__VA_ARGS__))(GEN_ARGTAIL,ARG0_OR_TAIL)(__VA_ARGS__) + +#endif + +#define VA_EAT(...) +#define VA_IDENT(...) __VA_ARGS__ +#define VA_ARG0_(A0,...) A0 +#define VA_ARG0(...) VA_ARG0_(__VA_ARGS__) +#define VA_ARGTAIL_(A0,...) __VA_ARGS__ +#define VA_ARGTAIL(...) VA_ARGTAIL_(__VA_ARGS__) + +// IFP helper macros to test IBP for IFN and IS_EMPTY +#define IFP_0(T,...) __VA_ARGS__ +#define IFP_1(T,...) T + +#define IFP_CAT(A,...) A##__VA_ARGS__ +#define IFP(BP) IFP_CAT(IFP_,BP) + +// IS_BEGIN_PAREN helper macros adapted from BOOST VMD +#define IBP_CAT_(A,...) A##__VA_ARGS__ +#define IBP_CAT(A,...) IBP_CAT_(A,__VA_ARGS__) + +#define IBP_ARG0_(A,...) A +#define IBP_ARG0(...) IBP_ARG0_(__VA_ARGS__) + +#define IBP_IS_ARGS(...) 1 + +#define IBP_1 1, +#define IBP_IBP_IS_ARGS 0, + +// IBP IS_BEGIN_PAREN returns 1 or 0 if ... ARGS is parenthesised +#define IBP(...) IBP_ARG0(IBP_CAT(IBP_, IBP_IS_ARGS __VA_ARGS__)) + +// IFN, IFE, IFNE and IF_EMPTY helpers without __VA_OPT__ support +#if ! VA_OPT_SUPPORT(?) + +# define IBP_(T,...) IBP_ARG0(IBP_CAT(IF##T##_, IBP_IS_ARGS __VA_ARGS__)) + + // IS_EMPTY helper macros, depend on IBP +# define IE_REDUCE_IBP(...) () +# define IE_GEN_0(...) 0 +# define IE_IBP(...) IBP(IE_REDUCE_IBP __VA_ARGS__ ()) + +# define GEN_IDENT(...) VA_IDENT +# define GEN_EAT(...) VA_EAT +# define GEN_ARGTAIL(...) VA_ARGTAIL +# define GEN_ARG0(...) VA_ARG0 + + // IFN, IFE, IFNE helper macros +# define EAT_OR_IDENT(...) IBP_(N,IE_REDUCE_IBP __VA_ARGS__ ()) +# define IFN_1 VA_EAT, +# define IFN_IBP_IS_ARGS VA_IDENT, + +# define IDENT_OR_EAT(...) IBP_(E,IE_REDUCE_IBP __VA_ARGS__ ()) +# define IFE_1 VA_IDENT, +# define IFE_IBP_IS_ARGS VA_EAT, + +# define ARG0_OR_TAIL(...) IBP_(NE,IE_REDUCE_IBP __VA_ARGS__ ()) +# define IFNE_1 VA_ARGTAIL, +# define IFNE_IBP_IS_ARGS VA_ARG0, + +#endif // IFN and IF_EMPTY defs diff --git a/src/rust/include/tracing.h b/src/rust/include/tracing.h index ce72976dd..c96afeff5 100644 --- a/src/rust/include/tracing.h +++ b/src/rust/include/tracing.h @@ -6,6 +6,7 @@ #define ZCASH_RUST_INCLUDE_TRACING_H #include "rust/types.h" +#include "rust/VA_OPT.hpp" #include "tracing/map.h" #include @@ -117,8 +118,8 @@ void tracing_log( #define T_FIELD_NAME(x, y) x #define T_FIELD_VALUE(x, y) y -#define T_FIELD_NAMES(...) MAP_PAIR_LIST(T_FIELD_NAME, __VA_ARGS__) -#define T_FIELD_VALUES(...) MAP_PAIR_LIST(T_FIELD_VALUE, __VA_ARGS__) +#define T_FIELD_NAMES(...) IFN(__VA_ARGS__)(MAP_PAIR_LIST(T_FIELD_NAME, __VA_ARGS__)) +#define T_FIELD_VALUES(...) IFN(__VA_ARGS__)(MAP_PAIR_LIST(T_FIELD_VALUE, __VA_ARGS__)) #define T_DOUBLEESCAPE(a) #a #define T_ESCAPEQUOTE(a) T_DOUBLEESCAPE(a) @@ -251,22 +252,6 @@ public: }; } // namespace tracing -/// Expands to a `tracing::Span` object which is used to record a span. -/// The `Span::Enter` method on that object records that the span has been -/// entered, and returns a RAII guard object, which will exit the span when -/// dropped. -/// -/// level, target, and name MUST be static constants, and MUST be valid UTF-8 -/// strings. -#define TracingSpan(level, target, name) ([&] { \ - static constexpr const char* const FIELDS[] = {}; \ - const char* T_VALUES[] = {}; \ - static TracingCallsite* CALLSITE = \ - T_CALLSITE(name, target, level, FIELDS, true); \ - return tracing::Span( \ - CALLSITE, T_VALUES, T_ARRLEN(T_VALUES)); \ -}()) - /// Expands to a `tracing::Span` object which is used to record a span. /// The `Span::Enter` method on that object records that the span has been /// entered, and returns a RAII guard object, which will exit the span when @@ -276,15 +261,15 @@ public: /// /// level, target, name, and all keys MUST be static constants, and MUST be /// valid UTF-8 strings. -#define TracingSpanFields(level, target, name, ...) ([&] { \ - static constexpr const char* const FIELDS[] = \ - {T_FIELD_NAMES(__VA_ARGS__)}; \ - const char* T_VALUES[] = \ - {T_FIELD_VALUES(__VA_ARGS__)}; \ - static TracingCallsite* CALLSITE = \ - T_CALLSITE(name, target, level, FIELDS, true); \ - return tracing::Span( \ - CALLSITE, T_VALUES, T_ARRLEN(T_VALUES)); \ +#define TracingSpan(level, target, name, ...) ([&] { \ + static constexpr const char* const FIELDS[] = \ + {T_FIELD_NAMES(__VA_ARGS__)}; \ + const char* T_VALUES[] = \ + {T_FIELD_VALUES(__VA_ARGS__)}; \ + static TracingCallsite* CALLSITE = \ + T_CALLSITE(name, target, level, FIELDS, true); \ + return tracing::Span( \ + CALLSITE, T_VALUES, T_ARRLEN(T_VALUES)); \ }()) #endif From 7a96af82602bcfd06b7c91ec98f678381d0ea4dd Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 24 Dec 2020 15:25:14 +0000 Subject: [PATCH 06/27] rust: Move helper macros into rust/helpers.h --- contrib/debian/copyright | 8 +++--- src/rust/include/rust/helpers.h | 34 ++++++++++++++++++++++++ src/rust/include/{tracing => rust}/map.h | 0 src/rust/include/tracing.h | 22 +-------------- 4 files changed, 39 insertions(+), 25 deletions(-) create mode 100644 src/rust/include/rust/helpers.h rename src/rust/include/{tracing => rust}/map.h (100%) diff --git a/contrib/debian/copyright b/contrib/debian/copyright index 8ec680585..d2c2fbc58 100644 --- a/contrib/debian/copyright +++ b/contrib/debian/copyright @@ -124,6 +124,10 @@ Files: src/crypto/ctaes/* Copyright: Copyright (c) 2016 Pieter Wuille License: Expat +Files: src/rust/include/rust/map.h +Copyright: Copyright (c) 2012 William Swanson +License: Expat-with-advertising-clause + Files: src/rust/include/rust/VA_OPT.hpp Copyright: Copyright (c) 2019 Will Wray License: Boost-Software-License-1.0 @@ -133,10 +137,6 @@ Files: src/rust/include/tracing.h Copyright: Copyright (c) 2020 Jack Grigg License: Expat -Files: src/rust/include/tracing/map.h -Copyright: Copyright (c) 2012 William Swanson -License: Expat-with-advertising-clause - Files: src/secp256k1/* Copyright: Copyright (c) 2013 Pieter Wuille License: Expat diff --git a/src/rust/include/rust/helpers.h b/src/rust/include/rust/helpers.h new file mode 100644 index 000000000..2383efe76 --- /dev/null +++ b/src/rust/include/rust/helpers.h @@ -0,0 +1,34 @@ +// Copyright (c) 2020 Jack Grigg +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php . + +#ifndef ZCASH_RUST_INCLUDE_HELPERS_H +#define ZCASH_RUST_INCLUDE_HELPERS_H + +#include "rust/map.h" +#include "rust/VA_OPT.hpp" + +// +// Helper macros +// + +#define MAP_PAIR_LIST0(f, x, y, peek, ...) f(x, y) MAP_LIST_NEXT(peek, MAP_PAIR_LIST1)(f, peek, __VA_ARGS__) +#define MAP_PAIR_LIST1(f, x, y, peek, ...) f(x, y) MAP_LIST_NEXT(peek, MAP_PAIR_LIST0)(f, peek, __VA_ARGS__) + +/// Applies the function macro `f` to each pair of the remaining parameters and +/// inserts commas between the results. +#define MAP_PAIR_LIST(f, ...) EVAL(MAP_PAIR_LIST1(f, __VA_ARGS__, ()()(), ()()(), ()()(), 0)) + +#define T_FIELD_NAME(x, y) x +#define T_FIELD_VALUE(x, y) y + +#define T_FIELD_NAMES(...) IFN(__VA_ARGS__)(MAP_PAIR_LIST(T_FIELD_NAME, __VA_ARGS__)) +#define T_FIELD_VALUES(...) IFN(__VA_ARGS__)(MAP_PAIR_LIST(T_FIELD_VALUE, __VA_ARGS__)) + +#define T_DOUBLEESCAPE(a) #a +#define T_ESCAPEQUOTE(a) T_DOUBLEESCAPE(a) + +// Computes the length of the given array. This is COUNT_OF from Chromium. +#define T_ARRLEN(x) ((sizeof(x) / sizeof(0 [x])) / ((size_t)(!(sizeof(x) % sizeof(0 [x]))))) + +#endif // ZCASH_RUST_INCLUDE_HELPERS_H diff --git a/src/rust/include/tracing/map.h b/src/rust/include/rust/map.h similarity index 100% rename from src/rust/include/tracing/map.h rename to src/rust/include/rust/map.h diff --git a/src/rust/include/tracing.h b/src/rust/include/tracing.h index c96afeff5..178204595 100644 --- a/src/rust/include/tracing.h +++ b/src/rust/include/tracing.h @@ -5,9 +5,8 @@ #ifndef ZCASH_RUST_INCLUDE_TRACING_H #define ZCASH_RUST_INCLUDE_TRACING_H +#include "rust/helpers.h" #include "rust/types.h" -#include "rust/VA_OPT.hpp" -#include "tracing/map.h" #include #include @@ -108,25 +107,6 @@ void tracing_log( // Helper macros // -#define MAP_PAIR_LIST0(f, x, y, peek, ...) f(x, y) MAP_LIST_NEXT(peek, MAP_PAIR_LIST1)(f, peek, __VA_ARGS__) -#define MAP_PAIR_LIST1(f, x, y, peek, ...) f(x, y) MAP_LIST_NEXT(peek, MAP_PAIR_LIST0)(f, peek, __VA_ARGS__) - -/// Applies the function macro `f` to each pair of the remaining parameters and -/// inserts commas between the results. -#define MAP_PAIR_LIST(f, ...) EVAL(MAP_PAIR_LIST1(f, __VA_ARGS__, ()()(), ()()(), ()()(), 0)) - -#define T_FIELD_NAME(x, y) x -#define T_FIELD_VALUE(x, y) y - -#define T_FIELD_NAMES(...) IFN(__VA_ARGS__)(MAP_PAIR_LIST(T_FIELD_NAME, __VA_ARGS__)) -#define T_FIELD_VALUES(...) IFN(__VA_ARGS__)(MAP_PAIR_LIST(T_FIELD_VALUE, __VA_ARGS__)) - -#define T_DOUBLEESCAPE(a) #a -#define T_ESCAPEQUOTE(a) T_DOUBLEESCAPE(a) - -// Computes the length of the given array. This is COUNT_OF from Chromium. -#define T_ARRLEN(x) ((sizeof(x) / sizeof(0 [x])) / ((size_t)(!(sizeof(x) % sizeof(0 [x]))))) - #ifdef __cplusplus // Constructs a tracing callsite. // From 92e75de46f75d879a3ca5c9ed8a1a3b15ed5adf2 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 24 Dec 2020 16:33:33 +0000 Subject: [PATCH 07/27] metrics: Add support for labels Given that label values may be dynamic, any metrics callsite with labels can't be stored statically. Instead, we contruct a fresh metrics Key each time we hit a metric with labels (if a metrics recorder is installed). --- src/rust/include/rust/metrics.h | 167 ++++++++++++++++++++++++++------ src/rust/src/metrics_ffi.rs | 106 ++++++++++++++++++-- 2 files changed, 235 insertions(+), 38 deletions(-) diff --git a/src/rust/include/rust/metrics.h b/src/rust/include/rust/metrics.h index 13aa23bb6..92fb7ebf8 100644 --- a/src/rust/include/rust/metrics.h +++ b/src/rust/include/rust/metrics.h @@ -5,6 +5,9 @@ #ifndef ZCASH_RUST_INCLUDE_RUST_METRICS_H #define ZCASH_RUST_INCLUDE_RUST_METRICS_H +#include "rust/helpers.h" + +#include #include #ifdef __cplusplus @@ -22,6 +25,9 @@ bool metrics_run(const char* listen_address); struct MetricsCallsite; typedef struct MetricsCallsite MetricsCallsite; +struct MetricsKey; +typedef struct MetricsKey MetricsKey; + /// Creates a metrics callsite. /// /// You should usually call one of the helper macros such as `MetricsCounter` @@ -32,36 +38,85 @@ typedef struct MetricsCallsite MetricsCallsite; /// UTF-8. MetricsCallsite* metrics_callsite(const char* name); +/// Creates a metrics key. +/// +/// This API supports labels that may not have static values, and is intended +/// to be called for each metrics callsite invocation. As such, it returns null +/// if a metrics recorder is not installed, to save on construction costs. +/// +/// You should usually call one of the helper macros such as `MetricsCounter` +/// instead of calling this directly. +/// +/// API requirements: +/// - label_names and label_values, if not null, MUST be the same length. +/// - All string arguments MUST be valid UTF-8. +MetricsKey* metrics_key( + const char* name, + const char* const* label_names, + const char* const* label_values, + size_t labels_len); + /// Increments a counter. /// /// Counters represent a single monotonic value, which means the value can only /// be incremented, not decremented, and always starts out with an initial value /// of zero. -void metrics_increment_counter(const MetricsCallsite* callsite, uint64_t value); +void metrics_static_increment_counter(const MetricsCallsite* callsite, uint64_t value); + +/// Increments a counter. +/// +/// Counters represent a single monotonic value, which means the value can only +/// be incremented, not decremented, and always starts out with an initial value +/// of zero. +void metrics_increment_counter(MetricsKey* key, uint64_t value); /// Updates a gauge. /// /// Gauges represent a single value that can go up or down over time, and always /// starts out with an initial value of zero. -void metrics_update_gauge(const MetricsCallsite* callsite, double value); +void metrics_static_update_gauge(const MetricsCallsite* callsite, double value); + +/// Updates a gauge. +/// +/// Gauges represent a single value that can go up or down over time, and always +/// starts out with an initial value of zero. +void metrics_update_gauge(MetricsKey* callsite, double value); /// Increments a gauge. /// /// Gauges represent a single value that can go up or down over time, and always /// starts out with an initial value of zero. -void metrics_increment_gauge(const MetricsCallsite* callsite, double value); +void metrics_static_increment_gauge(const MetricsCallsite* callsite, double value); + +/// Increments a gauge. +/// +/// Gauges represent a single value that can go up or down over time, and always +/// starts out with an initial value of zero. +void metrics_increment_gauge(MetricsKey* callsite, double value); /// Decrements a gauge. /// /// Gauges represent a single value that can go up or down over time, and always /// starts out with an initial value of zero. -void metrics_decrement_gauge(const MetricsCallsite* callsite, double value); +void metrics_static_decrement_gauge(const MetricsCallsite* callsite, double value); + +/// Decrements a gauge. +/// +/// Gauges represent a single value that can go up or down over time, and always +/// starts out with an initial value of zero. +void metrics_decrement_gauge(MetricsKey* callsite, double value); /// Records a histogram. /// /// Histograms measure the distribution of values for a given set of /// measurements, and start with no initial values. -void metrics_record_histogram(const MetricsCallsite* callsite, double value); +void metrics_static_record_histogram(const MetricsCallsite* callsite, double value); + +/// Records a histogram. +/// +/// Histograms measure the distribution of values for a given set of +/// measurements, and start with no initial values. +void metrics_record_histogram(MetricsKey* callsite, double value); #ifdef __cplusplus } @@ -90,6 +145,14 @@ void metrics_record_histogram(const MetricsCallsite* callsite, double value); metrics_callsite(name) #endif +// Constructs a metrics key. +#define M_KEY(name, labels, values) \ + metrics_key( \ + name, \ + labels, \ + values, \ + T_ARRLEN(labels)) + // // Metrics // @@ -100,11 +163,19 @@ void metrics_record_histogram(const MetricsCallsite* callsite, double value); /// be incremented, not decremented, and always starts out with an initial value /// of zero. /// -/// name MUST be a static constant, and MUST be a valid UTF-8 string. -#define MetricsCounter(name, value) \ - do { \ - static MetricsCallsite* CALLSITE = M_CALLSITE(name); \ - metrics_increment_counter(CALLSITE, value); \ +/// name MUST be a static constant, and all strings MUST be valid UTF-8. +#define MetricsCounter(name, value, ...) \ + do { \ + IFE(__VA_ARGS__) \ + (static MetricsCallsite* CALLSITE = M_CALLSITE(name); \ + metrics_static_increment_counter(CALLSITE, value);) \ + IFN(__VA_ARGS__)(const char* M_LABELS[] = \ + {T_FIELD_NAMES(__VA_ARGS__)}; \ + const char* M_VALUES[] = \ + {T_FIELD_VALUES(__VA_ARGS__)}; \ + MetricsKey* KEY = \ + M_KEY(name, M_LABELS, M_VALUES); \ + metrics_increment_counter(KEY, value);) \ } while (0) /// Increments a counter by one. @@ -113,19 +184,27 @@ void metrics_record_histogram(const MetricsCallsite* callsite, double value); /// be incremented, not decremented, and always starts out with an initial value /// of zero. /// -/// name MUST be a static constant, and MUST be a valid UTF-8 string. -#define MetricsIncrementCounter(name) MetricsCounter(name, 1) +/// name MUST be a static constant, and all strings MUST be valid UTF-8. +#define MetricsIncrementCounter(name, ...) MetricsCounter(name, 1, __VA_ARGS__) /// Updates a gauge. /// /// Gauges represent a single value that can go up or down over time, and always /// starts out with an initial value of zero. /// -/// name MUST be a static constant, and MUST be a valid UTF-8 string. -#define MetricsGauge(name, value) \ - do { \ - static MetricsCallsite* CALLSITE = M_CALLSITE(name); \ - metrics_update_gauge(CALLSITE, value); \ +/// name MUST be a static constant, and all strings MUST be valid UTF-8. +#define MetricsGauge(name, value, ...) \ + do { \ + IFE(__VA_ARGS__) \ + (static MetricsCallsite* CALLSITE = M_CALLSITE(name); \ + metrics_static_update_gauge(CALLSITE, value);) \ + IFN(__VA_ARGS__)(const char* M_LABELS[] = \ + {T_FIELD_NAMES(__VA_ARGS__)}; \ + const char* M_VALUES[] = \ + {T_FIELD_VALUES(__VA_ARGS__)}; \ + MetricsKey* KEY = \ + M_KEY(name, M_LABELS, M_VALUES); \ + metrics_update_gauge(KEY, value);) \ } while (0) /// Increments a gauge. @@ -133,11 +212,19 @@ void metrics_record_histogram(const MetricsCallsite* callsite, double value); /// Gauges represent a single value that can go up or down over time, and always /// starts out with an initial value of zero. /// -/// name MUST be a static constant, and MUST be a valid UTF-8 string. -#define MetricsIncrementGauge(name, value) \ - do { \ - static MetricsCallsite* CALLSITE = M_CALLSITE(name); \ - metrics_increment_gauge(CALLSITE, value); \ +/// name MUST be a static constant, and all strings MUST be valid UTF-8. +#define MetricsIncrementGauge(name, value, ...) \ + do { \ + IFE(__VA_ARGS__) \ + (static MetricsCallsite* CALLSITE = M_CALLSITE(name); \ + metrics_static_increment_gauge(CALLSITE, value);) \ + IFN(__VA_ARGS__)(const char* M_LABELS[] = \ + {T_FIELD_NAMES(__VA_ARGS__)}; \ + const char* M_VALUES[] = \ + {T_FIELD_VALUES(__VA_ARGS__)}; \ + MetricsKey* KEY = \ + M_KEY(name, M_LABELS, M_VALUES); \ + metrics_increment_gauge(KEY, value);) \ } while (0) /// Decrements a gauge. @@ -145,11 +232,19 @@ void metrics_record_histogram(const MetricsCallsite* callsite, double value); /// Gauges represent a single value that can go up or down over time, and always /// starts out with an initial value of zero. /// -/// name MUST be a static constant, and MUST be a valid UTF-8 string. -#define MetricsDecrementGauge(name, value) \ - do { \ - static MetricsCallsite* CALLSITE = M_CALLSITE(name); \ - metrics_decrement_gauge(CALLSITE, value); \ +/// name MUST be a static constant, and all strings MUST be valid UTF-8. +#define MetricsDecrementGauge(name, value, ...) \ + do { \ + IFE(__VA_ARGS__) \ + (static MetricsCallsite* CALLSITE = M_CALLSITE(name); \ + metrics_static_decrement_gauge(CALLSITE, value);) \ + IFN(__VA_ARGS__)(const char* M_LABELS[] = \ + {T_FIELD_NAMES(__VA_ARGS__)}; \ + const char* M_VALUES[] = \ + {T_FIELD_VALUES(__VA_ARGS__)}; \ + MetricsKey* KEY = \ + M_KEY(name, M_LABELS, M_VALUES); \ + metrics_decrement_gauge(KEY, value);) \ } while (0) /// Records a histogram. @@ -157,11 +252,19 @@ void metrics_record_histogram(const MetricsCallsite* callsite, double value); /// Histograms measure the distribution of values for a given set of /// measurements, and start with no initial values. /// -/// name MUST be a static constant, and MUST be a valid UTF-8 string. -#define MetricsHistogram(name, value) \ - do { \ - static MetricsCallsite* CALLSITE = M_CALLSITE(name); \ - metrics_record_histogram(CALLSITE, value); \ +/// name MUST be a static constant, and all strings MUST be valid UTF-8. +#define MetricsHistogram(name, value, ...) \ + do { \ + IFE(__VA_ARGS__) \ + (static MetricsCallsite* CALLSITE = M_CALLSITE(name); \ + metrics_static_record_histogram(CALLSITE, value);) \ + IFN(__VA_ARGS__)(const char* M_LABELS[] = \ + {T_FIELD_NAMES(__VA_ARGS__)}; \ + const char* M_VALUES[] = \ + {T_FIELD_VALUES(__VA_ARGS__)}; \ + MetricsKey* KEY = \ + M_KEY(name, M_LABELS, M_VALUES); \ + metrics_record_histogram(KEY, value);) \ } while (0) #endif // ZCASH_RUST_INCLUDE_RUST_METRICS_H diff --git a/src/rust/src/metrics_ffi.rs b/src/rust/src/metrics_ffi.rs index eecdcdfc2..ba4d37ef0 100644 --- a/src/rust/src/metrics_ffi.rs +++ b/src/rust/src/metrics_ffi.rs @@ -1,8 +1,10 @@ use libc::{c_char, c_double}; -use metrics::{try_recorder, GaugeValue, Key, KeyData}; +use metrics::{try_recorder, GaugeValue, Key, KeyData, Label}; use metrics_exporter_prometheus::PrometheusBuilder; use std::ffi::CStr; use std::net::SocketAddr; +use std::ptr; +use std::slice; use tracing::error; #[no_mangle] @@ -41,8 +43,50 @@ pub extern "C" fn metrics_callsite(name: *const c_char) -> *mut FfiCallsite { })) } +pub struct FfiKey { + inner: Key, +} + #[no_mangle] -pub extern "C" fn metrics_increment_counter(callsite: *const FfiCallsite, value: u64) { +pub extern "C" fn metrics_key( + name: *const c_char, + label_names: *const *const c_char, + label_values: *const *const c_char, + labels_len: usize, +) -> *mut FfiKey { + if try_recorder().is_none() { + // No recorder is currently installed, so don't genenerate a key. We check for + // null inside each API that consumes an FfiKey, just in case a recorder was + // installed in a racy way. + ptr::null_mut() + } else { + let name = unsafe { CStr::from_ptr(name) }.to_str().unwrap(); + let labels = unsafe { slice::from_raw_parts(label_names, labels_len) }; + let values = unsafe { slice::from_raw_parts(label_values, labels_len) }; + + let stringify = |s: &[_]| { + s.iter() + .map(|&p| unsafe { CStr::from_ptr(p) }) + .map(|cs| cs.to_string_lossy().into_owned()) + .collect::>() + }; + let labels = stringify(labels); + let values = stringify(values); + + let labels: Vec<_> = labels + .into_iter() + .zip(values.into_iter()) + .map(|(name, value)| Label::new(name, value)) + .collect(); + + Box::into_raw(Box::new(FfiKey { + inner: Key::Owned(KeyData::from_parts(name, labels)), + })) + } +} + +#[no_mangle] +pub extern "C" fn metrics_static_increment_counter(callsite: *const FfiCallsite, value: u64) { if let Some(recorder) = try_recorder() { let callsite = unsafe { callsite.as_ref().unwrap() }; recorder.increment_counter(Key::Borrowed(&callsite.key_data), value); @@ -50,7 +94,17 @@ pub extern "C" fn metrics_increment_counter(callsite: *const FfiCallsite, value: } #[no_mangle] -pub extern "C" fn metrics_update_gauge(callsite: *const FfiCallsite, value: c_double) { +pub extern "C" fn metrics_increment_counter(key: *mut FfiKey, value: u64) { + if let Some(recorder) = try_recorder() { + if !key.is_null() { + let key = unsafe { Box::from_raw(key) }; + recorder.increment_counter(key.inner, value); + } + } +} + +#[no_mangle] +pub extern "C" fn metrics_static_update_gauge(callsite: *const FfiCallsite, value: c_double) { if let Some(recorder) = try_recorder() { let callsite = unsafe { callsite.as_ref().unwrap() }; recorder.update_gauge( @@ -61,7 +115,17 @@ pub extern "C" fn metrics_update_gauge(callsite: *const FfiCallsite, value: c_do } #[no_mangle] -pub extern "C" fn metrics_increment_gauge(callsite: *const FfiCallsite, value: c_double) { +pub extern "C" fn metrics_update_gauge(key: *mut FfiKey, value: c_double) { + if let Some(recorder) = try_recorder() { + if !key.is_null() { + let key = unsafe { Box::from_raw(key) }; + recorder.update_gauge(key.inner, GaugeValue::Absolute(value)); + } + } +} + +#[no_mangle] +pub extern "C" fn metrics_static_increment_gauge(callsite: *const FfiCallsite, value: c_double) { if let Some(recorder) = try_recorder() { let callsite = unsafe { callsite.as_ref().unwrap() }; recorder.update_gauge( @@ -72,7 +136,17 @@ pub extern "C" fn metrics_increment_gauge(callsite: *const FfiCallsite, value: c } #[no_mangle] -pub extern "C" fn metrics_decrement_gauge(callsite: *const FfiCallsite, value: c_double) { +pub extern "C" fn metrics_increment_gauge(key: *mut FfiKey, value: c_double) { + if let Some(recorder) = try_recorder() { + if !key.is_null() { + let key = unsafe { Box::from_raw(key) }; + recorder.update_gauge(key.inner, GaugeValue::Increment(value)); + } + } +} + +#[no_mangle] +pub extern "C" fn metrics_static_decrement_gauge(callsite: *const FfiCallsite, value: c_double) { if let Some(recorder) = try_recorder() { let callsite = unsafe { callsite.as_ref().unwrap() }; recorder.update_gauge( @@ -83,9 +157,29 @@ pub extern "C" fn metrics_decrement_gauge(callsite: *const FfiCallsite, value: c } #[no_mangle] -pub extern "C" fn metrics_record_histogram(callsite: *const FfiCallsite, value: c_double) { +pub extern "C" fn metrics_decrement_gauge(key: *mut FfiKey, value: c_double) { + if let Some(recorder) = try_recorder() { + if !key.is_null() { + let key = unsafe { Box::from_raw(key) }; + recorder.update_gauge(key.inner, GaugeValue::Decrement(value)); + } + } +} + +#[no_mangle] +pub extern "C" fn metrics_static_record_histogram(callsite: *const FfiCallsite, value: c_double) { if let Some(recorder) = try_recorder() { let callsite = unsafe { callsite.as_ref().unwrap() }; recorder.record_histogram(Key::Borrowed(&callsite.key_data), value); } } + +#[no_mangle] +pub extern "C" fn metrics_record_histogram(key: *mut FfiKey, value: c_double) { + if let Some(recorder) = try_recorder() { + if !key.is_null() { + let key = unsafe { Box::from_raw(key) }; + recorder.record_histogram(key.inner, value); + } + } +} From 1c2c8fed2e6c2bcf71219b6face72975898a483d Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 24 Dec 2020 16:33:44 +0000 Subject: [PATCH 08/27] metrics: Expose binary metadata --- src/init.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/init.cpp b/src/init.cpp index df6818dfa..101f03165 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1236,6 +1236,12 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) } } + // Expose binary metadata to metrics, using a single time series with value 1. + // https://www.robustperception.io/exposing-the-software-version-to-prometheus + MetricsIncrementCounter( + "zcashd.build.info", + "version", CLIENT_BUILD.c_str()); + if ((chainparams.NetworkIDString() != "regtest") && GetBoolArg("-showmetrics", isatty(STDOUT_FILENO)) && !fPrintToConsole && !GetBoolArg("-daemon", false)) { From ab3196c1dfaf4fd4bea16615d8c96ea1d22551e4 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 5 Jan 2021 01:08:22 +0000 Subject: [PATCH 09/27] Add more detailed metrics - Sprout and Sapling pool metrics (commitments, values) - Block verification time histogram (as a summary, not bucketed) - Mempool stats (same as getmempoolinfo returns) - Inbound and outbound bytes labelled by command - Added command labels to message counters --- src/main.cpp | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/net.cpp | 16 +++++++++++++--- src/net.h | 1 + 3 files changed, 66 insertions(+), 3 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 9a21065de..cf4bb62a4 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1693,6 +1693,10 @@ bool AcceptToMemoryPool( } pool.EnsureSizeLimit(); + + MetricsGauge("mempool.size.transactions", mempool.size()); + MetricsGauge("mempool.size.bytes", mempool.GetTotalTxSize()); + MetricsGauge("mempool.usage.bytes", mempool.DynamicMemoryUsage()); } } @@ -3212,6 +3216,47 @@ void PruneAndFlush() { FlushStateToDisk(Params(), state, FLUSH_STATE_NONE); } +struct PoolMetrics { + size_t commitments; + std::optional value; + + static PoolMetrics Sprout(CBlockIndex *pindex, CCoinsViewCache *view) { + PoolMetrics stats; + stats.value = pindex->nChainSproutValue; + + SproutMerkleTree sproutTree; + assert(view->GetSproutAnchorAt(pindex->hashFinalSproutRoot, sproutTree)); + stats.commitments = sproutTree.size(); + + return stats; + } + + static PoolMetrics Sapling(CBlockIndex *pindex, CCoinsViewCache *view) { + PoolMetrics stats; + stats.value = pindex->nChainSaplingValue; + + // Before Sapling activation, stats.commitments will be zero. + SaplingMerkleTree saplingTree; + if (view->GetSaplingAnchorAt(pindex->hashFinalSaplingRoot, saplingTree)) { + stats.commitments = saplingTree.size(); + } + + return stats; + } +}; + +#define RenderPoolMetrics(poolName, poolMetrics) \ + do { \ + static constexpr const char* poolCommitments = \ + "pool." poolName ".commitments"; \ + static constexpr const char* poolValue = \ + "pool." poolName ".value.zatoshis"; \ + MetricsGauge(poolCommitments, poolMetrics.commitments); \ + if (poolMetrics.value) { \ + MetricsGauge(poolValue, poolMetrics.value.value()); \ + } \ + } while (0) + /** Update chainActive and related internal data structures. */ void static UpdateTip(CBlockIndex *pindexNew, const CChainParams& chainParams) { chainActive.SetTip(pindexNew); @@ -3238,7 +3283,13 @@ void static UpdateTip(CBlockIndex *pindexNew, const CChainParams& chainParams) { "date", date.c_str(), "progress", progress.c_str(), "cache", cache.c_str()); + + auto sproutPool = PoolMetrics::Sprout(pindexNew, pcoinsTip); + auto saplingPool = PoolMetrics::Sapling(pindexNew, pcoinsTip); + MetricsGauge("block.verified.block.height", pindexNew->nHeight); + RenderPoolMetrics("sprout", sproutPool); + RenderPoolMetrics("sapling", saplingPool); cvBlockChange.notify_all(); } @@ -3372,6 +3423,7 @@ bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, LogPrint("bench", " - Connect postprocess: %.2fms [%.2fs]\n", (nTime6 - nTime5) * 0.001, nTimePostConnect * 0.000001); LogPrint("bench", "- Connect block: %.2fms [%.2fs]\n", (nTime6 - nTime1) * 0.001, nTimeTotal * 0.000001); MetricsIncrementCounter("block.verified.block.count"); + MetricsHistogram("block.verified.block.seconds", (nTime6 - nTime1) * 0.000001); return true; } diff --git a/src/net.cpp b/src/net.cpp index f7d54aed6..ea0014b57 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -704,7 +704,11 @@ bool CNode::ReceiveMsgBytes(const char *pch, unsigned int nBytes) if (msg.complete()) { msg.nTime = GetTimeMicros(); - MetricsIncrementCounter("peer.inbound.messages"); + std::string strCommand = SanitizeString(msg.hdr.GetCommand()); + MetricsIncrementCounter("peer.inbound.messages", "command", strCommand.c_str()); + MetricsCounter( + "peer.inbound.bytes", msg.hdr.nMessageSize, + "command", strCommand.c_str()); messageHandlerCondition.notify_one(); } } @@ -2271,8 +2275,10 @@ void CNode::BeginMessage(const char* pszCommand) EXCLUSIVE_LOCK_FUNCTION(cs_vSen { ENTER_CRITICAL_SECTION(cs_vSend); assert(ssSend.size() == 0); + assert(strSendCommand.empty()); ssSend << CMessageHeader(Params().MessageStart(), pszCommand, 0); - LogPrint("net", "sending: %s ", SanitizeString(pszCommand)); + strSendCommand = SanitizeString(pszCommand); + LogPrint("net", "sending: %s ", strSendCommand); } void CNode::AbortMessage() UNLOCK_FUNCTION(cs_vSend) @@ -2286,7 +2292,7 @@ void CNode::AbortMessage() UNLOCK_FUNCTION(cs_vSend) void CNode::EndMessage() UNLOCK_FUNCTION(cs_vSend) { - MetricsIncrementCounter("peer.outbound.messages"); + MetricsIncrementCounter("peer.outbound.messages", "command", strSendCommand.c_str()); // The -*messagestest options are intentionally not documented in the help message, // since they are only used during development to debug the networking code and are // not intended for end-users. @@ -2320,6 +2326,10 @@ void CNode::EndMessage() UNLOCK_FUNCTION(cs_vSend) std::deque::iterator it = vSendMsg.insert(vSendMsg.end(), CSerializeData()); ssSend.GetAndClear(*it); nSendSize += (*it).size(); + MetricsCounter( + "peer.outbound.bytes", (*it).size(), + "command", strSendCommand.c_str()); + strSendCommand.clear(); // If write queue empty, attempt "optimistic write" if (it == vSendMsg.begin()) diff --git a/src/net.h b/src/net.h index f1949a1ee..f5a238d56 100644 --- a/src/net.h +++ b/src/net.h @@ -256,6 +256,7 @@ public: uint64_t nServices; SOCKET hSocket; CDataStream ssSend; + std::string strSendCommand; // Current command being assembled in ssSend size_t nSendSize; // total size of all vSendMsg entries size_t nSendOffset; // offset inside the first vSendMsg already sent uint64_t nSendBytes; From 523f96965468c540c8e925aeb3156e4abfd81a24 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 6 Jan 2021 15:28:42 +0000 Subject: [PATCH 10/27] rust: Use consistent include guards in header files --- src/rust/include/rust/VA_OPT.hpp | 6 +++++- src/rust/include/rust/helpers.h | 6 +++--- src/rust/include/rust/map.h | 7 ++++--- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/rust/include/rust/VA_OPT.hpp b/src/rust/include/rust/VA_OPT.hpp index 430282575..553b637ac 100644 --- a/src/rust/include/rust/VA_OPT.hpp +++ b/src/rust/include/rust/VA_OPT.hpp @@ -1,11 +1,13 @@ // Copyright (c) 2019 Will Wray https://keybase.io/willwray +// Copyright (c) 2020 The Zcash developers // // Distributed under the Boost Software License, Version 1.0. // http://www.boost.org/LICENSE_1_0.txt // // Repo: https://github.com/willwray/VA_OPT -#pragma once +#ifndef ZCASH_RUST_INCLUDE_RUST_VA_OPT_H +#define ZCASH_RUST_INCLUDE_RUST_VA_OPT_H /* VA_OPT.hpp @@ -150,3 +152,5 @@ # define IFNE_IBP_IS_ARGS VA_ARG0, #endif // IFN and IF_EMPTY defs + +#endif // ZCASH_RUST_INCLUDE_RUST_VA_OPT_H diff --git a/src/rust/include/rust/helpers.h b/src/rust/include/rust/helpers.h index 2383efe76..0bdaa113a 100644 --- a/src/rust/include/rust/helpers.h +++ b/src/rust/include/rust/helpers.h @@ -2,8 +2,8 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or https://www.opensource.org/licenses/mit-license.php . -#ifndef ZCASH_RUST_INCLUDE_HELPERS_H -#define ZCASH_RUST_INCLUDE_HELPERS_H +#ifndef ZCASH_RUST_INCLUDE_RUST_HELPERS_H +#define ZCASH_RUST_INCLUDE_RUST_HELPERS_H #include "rust/map.h" #include "rust/VA_OPT.hpp" @@ -31,4 +31,4 @@ // Computes the length of the given array. This is COUNT_OF from Chromium. #define T_ARRLEN(x) ((sizeof(x) / sizeof(0 [x])) / ((size_t)(!(sizeof(x) % sizeof(0 [x]))))) -#endif // ZCASH_RUST_INCLUDE_HELPERS_H +#endif // ZCASH_RUST_INCLUDE_RUST_HELPERS_H diff --git a/src/rust/include/rust/map.h b/src/rust/include/rust/map.h index 45f572ed2..efdbbb7f2 100644 --- a/src/rust/include/rust/map.h +++ b/src/rust/include/rust/map.h @@ -1,5 +1,6 @@ /* * Copyright (C) 2012 William Swanson + * Copyright (C) 2020 The Zcash developers * * Permission is hereby granted, free of charge, to any person * obtaining a copy of this software and associated documentation @@ -26,8 +27,8 @@ * prior written authorization from the authors. */ -#ifndef ZCASH_RUST_INCLUDE_TRACING_MAP_H -#define ZCASH_RUST_INCLUDE_TRACING_MAP_H +#ifndef ZCASH_RUST_INCLUDE_RUST_MAP_H +#define ZCASH_RUST_INCLUDE_RUST_MAP_H #define EVAL0(...) __VA_ARGS__ #define EVAL1(...) EVAL0(EVAL0(EVAL0(__VA_ARGS__))) @@ -67,4 +68,4 @@ */ #define MAP_LIST(f, ...) EVAL(MAP_LIST1(f, __VA_ARGS__, ()()(), ()()(), ()()(), 0)) -#endif // ZCASH_RUST_INCLUDE_TRACING_MAP_H +#endif // ZCASH_RUST_INCLUDE_RUST_MAP_H From d0f468e1ce05ce7a5aa66f733852ec02419aaea1 Mon Sep 17 00:00:00 2001 From: str4d Date: Thu, 7 Jan 2021 04:46:55 +1300 Subject: [PATCH 11/27] Add security warnings for -prometheusmetrics option Co-authored-by: Daira Hopwood --- contrib/metrics/README.md | 6 ++++++ src/init.cpp | 3 ++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/contrib/metrics/README.md b/contrib/metrics/README.md index 6135ce544..a95550741 100644 --- a/contrib/metrics/README.md +++ b/contrib/metrics/README.md @@ -30,6 +30,12 @@ You can see what each method provides with `zcash-cli help METHOD_NAME`. `zcashd` can optionally expose an HTTP server that acts as a Prometheus scrape endpoint. The server will respond to `GET` requests on any request path. +Note that HTTPS is not supported, and therefore connections to the endpoint are +not encrypted or authenticated. Access to the endpoint should be assumed to +compromise the privacy of node operations, by the provided metrics and/or by +timing side channels. Enabling the endpoint is **strongly discouraged** if the +node has a wallet holding live funds. + To enable the endpoint, add `-prometheusmetrics=:` to your `zcashd` configuration (either in `zcash.conf` or on the command line). After restarting `zcashd` you can then test the endpoint by querying it: diff --git a/src/init.cpp b/src/init.cpp index 101f03165..0225de67e 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -354,7 +354,8 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageOpt("-pid=", strprintf(_("Specify pid file (default: %s)"), BITCOIN_PID_FILENAME)); #endif strUsage += HelpMessageOpt("-prometheusmetrics=:", _("Expose node metrics in the Prometheus exposition format. " - "An HTTP listener will be started on the configured hostname and port, which responds to GET requests on any request path.")); + "An HTTP listener will be started on the configured hostname and port, which responds to GET requests on any request path. " + "SECURITY WARNING: this can potentially compromise privacy; read contrib/metrics/README.md before enabling.")); strUsage += HelpMessageOpt("-prune=", strprintf(_("Reduce storage requirements by pruning (deleting) old blocks. This mode disables wallet support and is incompatible with -txindex. " "Warning: Reverting this setting requires re-downloading the entire blockchain. " "(default: 0 = disable pruning blocks, >%u = target size in MiB to use for block files)"), MIN_DISK_SPACE_FOR_BLOCK_FILES / 1024 / 1024)); From dde7546091b28ae5d1a425b217c8abe4925681bf Mon Sep 17 00:00:00 2001 From: str4d Date: Thu, 7 Jan 2021 04:49:14 +1300 Subject: [PATCH 12/27] Clean up comment Co-authored-by: Daira Hopwood --- src/init.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 0225de67e..5b44b50f7 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1229,8 +1229,7 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) std::string prometheusMetricsArg = GetArg("-prometheusmetrics", ""); if (prometheusMetricsArg != "") { // Start up the metrics runtime. This spins off a Rust thread that runs - // the Prometheus exporter. We just let this thread die at process end - // ¯\_(ツ)_/¯ + // the Prometheus exporter. We just let this thread die at process end. LogPrintf("metrics thread start"); if (!metrics_run(prometheusMetricsArg.c_str())) { return InitError(strprintf(_("Failed to start Prometheus metrics exporter on '%s'"), prometheusMetricsArg)); From 1e5f9284c04a0b4d21efcd5e88aa10b9a10b38d1 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 6 Jan 2021 16:02:55 +0000 Subject: [PATCH 13/27] rust: Check for invalid UTF-8 in -prometheusmetrics argument --- src/rust/src/metrics_ffi.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/rust/src/metrics_ffi.rs b/src/rust/src/metrics_ffi.rs index ba4d37ef0..57c33ebd8 100644 --- a/src/rust/src/metrics_ffi.rs +++ b/src/rust/src/metrics_ffi.rs @@ -9,7 +9,13 @@ use tracing::error; #[no_mangle] pub extern "C" fn metrics_run(listen_address: *const c_char) -> bool { - let listen_address = unsafe { CStr::from_ptr(listen_address) }.to_str().unwrap(); + let listen_address = match unsafe { CStr::from_ptr(listen_address) }.to_str() { + Ok(addr) => addr, + Err(_) => { + error!("-prometheusmetrics argument is not valid UTF-8"); + return false; + } + }; listen_address .parse::() .map_err(|e| { From 34d2edb619fc27656e2ed579a92317a02ad225fa Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 6 Jan 2021 20:41:42 +0000 Subject: [PATCH 14/27] Add -prometheusmetrics to release notes --- doc/release-notes.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/doc/release-notes.md b/doc/release-notes.md index a29094b51..667aa0244 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -4,3 +4,20 @@ release-notes at release time) Notable changes =============== +Prometheus metrics +------------------ + +`zcashd` can now be configured to optionally expose an HTTP server that acts as +a Prometheus scrape endpoint. The server will respond to `GET` requests on any +request path. + +Note that HTTPS is not supported, and therefore connections to the endpoint are +not encrypted or authenticated. Access to the endpoint should be assumed to +compromise the privacy of node operations, by the provided metrics and/or by +timing side channels. Enabling the endpoint is **strongly discouraged** if the +node has a wallet holding live funds. + +To enable the endpoint, add `-prometheusmetrics=:` to your +`zcashd` configuration (either in `zcash.conf` or on the command line). After +restarting `zcashd` you can then test the endpoint by querying it with e.g. +`curl http://:`. From 59da774f22afe6246b9a7808e1ed2da198ba4a82 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 6 Jan 2021 21:03:47 +0000 Subject: [PATCH 15/27] Mention in release notes that metrics names may still change --- doc/release-notes.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/release-notes.md b/doc/release-notes.md index 667aa0244..4e3dde157 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -21,3 +21,6 @@ To enable the endpoint, add `-prometheusmetrics=:` to your `zcashd` configuration (either in `zcash.conf` or on the command line). After restarting `zcashd` you can then test the endpoint by querying it with e.g. `curl http://:`. + +The specific metrics names may change in subsequent releases, in particular to +improve interoperability with `zebrad`. From d08cdbe5f7c4ee482d193bf5dad411e9ac295d8e Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 7 Jan 2021 05:30:28 +0000 Subject: [PATCH 16/27] metrics: Implement IP access control on Prometheus scrape endpoint --- Cargo.lock | 10 +++ Cargo.toml | 4 + contrib/metrics/README.md | 20 +++-- doc/release-notes.md | 20 +++-- src/init.cpp | 32 +++++-- src/rust/include/rust/metrics.h | 10 ++- src/rust/src/metrics_ffi.rs | 80 +++++++++++------ src/rust/src/metrics_ffi/prometheus.rs | 113 +++++++++++++++++++++++++ src/util.cpp | 1 + 9 files changed, 237 insertions(+), 53 deletions(-) create mode 100644 src/rust/src/metrics_ffi/prometheus.rs diff --git a/Cargo.lock b/Cargo.lock index 22d4e80a2..766216186 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -676,6 +676,12 @@ dependencies = [ "cfg-if 1.0.0", ] +[[package]] +name = "ipnet" +version = "2.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "47be2f14c678be2fdcab04ab1171db51b2762ce6f0a8ee87c8dd4a04ed216135" + [[package]] name = "itoa" version = "0.4.7" @@ -719,12 +725,16 @@ dependencies = [ "ed25519-zebra", "funty", "group", + "hyper", + "ipnet", "jubjub", "libc", "metrics", "metrics-exporter-prometheus", "rand_core", "subtle", + "thiserror", + "tokio", "tracing", "tracing-appender", "tracing-core", diff --git a/Cargo.toml b/Cargo.toml index e4e2cfd69..86c225877 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,8 +38,12 @@ zcash_proofs = "0.5" ed25519-zebra = "2.0.0" # Metrics +hyper = { version = "0.14", default-features = false, features = ["server", "tcp", "http1"] } +ipnet = "2" metrics = "0.14.2" metrics-exporter-prometheus = "0.3" +thiserror = "1" +tokio = { version = "1.0", features = ["rt", "net", "time", "macros"] } # Temporary workaround for https://github.com/myrrlyn/funty/issues/3 funty = "=1.1.0" diff --git a/contrib/metrics/README.md b/contrib/metrics/README.md index a95550741..cfbb27c11 100644 --- a/contrib/metrics/README.md +++ b/contrib/metrics/README.md @@ -30,18 +30,12 @@ You can see what each method provides with `zcash-cli help METHOD_NAME`. `zcashd` can optionally expose an HTTP server that acts as a Prometheus scrape endpoint. The server will respond to `GET` requests on any request path. -Note that HTTPS is not supported, and therefore connections to the endpoint are -not encrypted or authenticated. Access to the endpoint should be assumed to -compromise the privacy of node operations, by the provided metrics and/or by -timing side channels. Enabling the endpoint is **strongly discouraged** if the -node has a wallet holding live funds. - -To enable the endpoint, add `-prometheusmetrics=:` to your -`zcashd` configuration (either in `zcash.conf` or on the command line). After +To enable the endpoint, add `-prometheusport=:` to your `zcashd` +configuration (either in `zcash.conf` or on the command line). After restarting `zcashd` you can then test the endpoint by querying it: ``` -$ curl http://: +$ curl http://127.0.0.1: # TYPE peer_outbound_messages counter peer_outbound_messages 181 @@ -59,6 +53,14 @@ block_verified_block_count 162 ... ``` +By default, access is restricted to localhost. This can be expanded with +`-metricsallowip=`, which can specify IPs or subnets. Note that HTTPS is not +supported, and therefore connections to the endpoint are not encrypted or +authenticated. Access to the endpoint should be assumed to compromise the +privacy of node operations, by the provided metrics and/or by timing side +channels. Non-localhost access is **strongly discouraged** if the node has a +wallet holding live funds. + ### Example metrics collection with Docker The example instructions below were tested on Windows 10 using Docker Desktop diff --git a/doc/release-notes.md b/doc/release-notes.md index 4e3dde157..8cce56b9c 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -11,16 +11,18 @@ Prometheus metrics a Prometheus scrape endpoint. The server will respond to `GET` requests on any request path. -Note that HTTPS is not supported, and therefore connections to the endpoint are -not encrypted or authenticated. Access to the endpoint should be assumed to -compromise the privacy of node operations, by the provided metrics and/or by -timing side channels. Enabling the endpoint is **strongly discouraged** if the -node has a wallet holding live funds. - -To enable the endpoint, add `-prometheusmetrics=:` to your -`zcashd` configuration (either in `zcash.conf` or on the command line). After +To enable the endpoint, add `-prometheusport=` to your `zcashd` +configuration (either in `zcash.conf` or on the command line). After restarting `zcashd` you can then test the endpoint by querying it with e.g. -`curl http://:`. +`curl http://127.0.0.1:`. + +By default, access is restricted to localhost. This can be expanded with +`-metricsallowip=`, which can specify IPs or subnets. Note that HTTPS is not +supported, and therefore connections to the endpoint are not encrypted or +authenticated. Access to the endpoint should be assumed to compromise the +privacy of node operations, by the provided metrics and/or by timing side +channels. Non-localhost access is **strongly discouraged** if the node has a +wallet holding live funds. The specific metrics names may change in subsequent releases, in particular to improve interoperability with `zebrad`. diff --git a/src/init.cpp b/src/init.cpp index 5b44b50f7..60876765b 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -353,9 +353,6 @@ std::string HelpMessage(HelpMessageMode mode) #ifndef WIN32 strUsage += HelpMessageOpt("-pid=", strprintf(_("Specify pid file (default: %s)"), BITCOIN_PID_FILENAME)); #endif - strUsage += HelpMessageOpt("-prometheusmetrics=:", _("Expose node metrics in the Prometheus exposition format. " - "An HTTP listener will be started on the configured hostname and port, which responds to GET requests on any request path. " - "SECURITY WARNING: this can potentially compromise privacy; read contrib/metrics/README.md before enabling.")); strUsage += HelpMessageOpt("-prune=", strprintf(_("Reduce storage requirements by pruning (deleting) old blocks. This mode disables wallet support and is incompatible with -txindex. " "Warning: Reverting this setting requires re-downloading the entire blockchain. " "(default: 0 = disable pruning blocks, >%u = target size in MiB to use for block files)"), MIN_DISK_SPACE_FOR_BLOCK_FILES / 1024 / 1024)); @@ -415,6 +412,15 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageOpt("-zmqpubrawtx=
", _("Enable publish raw transaction in
")); #endif + strUsage += HelpMessageGroup(_("Monitoring options:")); + strUsage += HelpMessageOpt("-metricsallowip=", _("Allow metrics connections from specified source. " + "Valid for are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0) or a network/CIDR (e.g. 1.2.3.4/24). " + "This option can be specified multiple times. (default: only localhost)")); + strUsage += HelpMessageOpt("-metricsbind=", _("Bind to given address to listen for metrics connections. (default: bind to all interfaces)")); + strUsage += HelpMessageOpt("-prometheusport=", _("Expose node metrics in the Prometheus exposition format. " + "An HTTP listener will be started on , which responds to GET requests on any request path. " + "Use -metricsallowip and -metricsbind to control access.")); + strUsage += HelpMessageGroup(_("Debugging/Testing options:")); if (showDebug) { @@ -1226,13 +1232,25 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) // Count uptime MarkStartTime(); - std::string prometheusMetricsArg = GetArg("-prometheusmetrics", ""); - if (prometheusMetricsArg != "") { + int prometheusPort = GetArg("-prometheusport", -1); + if (prometheusPort > 0) { + const std::vector& vAllow = mapMultiArgs["-metricsallowip"]; + std::vector vAllowCstr; + for (const std::string& strAllow : vAllow) { + vAllowCstr.push_back(strAllow.c_str()); + } + + std::string metricsBind = GetArg("-metricsbind", ""); + const char* metricsBindCstr = nullptr; + if (!metricsBind.empty()) { + metricsBindCstr = metricsBind.c_str(); + } + // Start up the metrics runtime. This spins off a Rust thread that runs // the Prometheus exporter. We just let this thread die at process end. LogPrintf("metrics thread start"); - if (!metrics_run(prometheusMetricsArg.c_str())) { - return InitError(strprintf(_("Failed to start Prometheus metrics exporter on '%s'"), prometheusMetricsArg)); + if (!metrics_run(metricsBindCstr, vAllowCstr.data(), vAllowCstr.size(), prometheusPort)) { + return InitError(strprintf(_("Failed to start Prometheus metrics exporter"))); } } diff --git a/src/rust/include/rust/metrics.h b/src/rust/include/rust/metrics.h index 92fb7ebf8..8c8c53c90 100644 --- a/src/rust/include/rust/metrics.h +++ b/src/rust/include/rust/metrics.h @@ -17,10 +17,14 @@ extern "C" { /// Initializes the metrics runtime and runs the Prometheus exporter in a new /// thread. /// -/// listen_address is a string like :. +/// bind_address is an IP address to bind to, or empty to use the default. /// -/// Returns false if listen_address was not a valid SocketAddr. -bool metrics_run(const char* listen_address); +/// Returns false on any error. +bool metrics_run( + const char* bind_address, + const char* const* allow_ips, + size_t allow_ips_len, + uint16_t prometheus_port); struct MetricsCallsite; typedef struct MetricsCallsite MetricsCallsite; diff --git a/src/rust/src/metrics_ffi.rs b/src/rust/src/metrics_ffi.rs index 57c33ebd8..1c215f9c0 100644 --- a/src/rust/src/metrics_ffi.rs +++ b/src/rust/src/metrics_ffi.rs @@ -2,39 +2,69 @@ use libc::{c_char, c_double}; use metrics::{try_recorder, GaugeValue, Key, KeyData, Label}; use metrics_exporter_prometheus::PrometheusBuilder; use std::ffi::CStr; -use std::net::SocketAddr; +use std::net::{IpAddr, SocketAddr}; use std::ptr; use std::slice; use tracing::error; +mod prometheus; + #[no_mangle] -pub extern "C" fn metrics_run(listen_address: *const c_char) -> bool { - let listen_address = match unsafe { CStr::from_ptr(listen_address) }.to_str() { - Ok(addr) => addr, - Err(_) => { - error!("-prometheusmetrics argument is not valid UTF-8"); +pub extern "C" fn metrics_run( + bind_address: *const c_char, + allow_ips: *const *const c_char, + allow_ips_len: usize, + prometheus_port: u16, +) -> bool { + // Parse any allowed IPs. + let allow_ips = unsafe { slice::from_raw_parts(allow_ips, allow_ips_len) }; + let mut allow_ips: Vec = match allow_ips + .iter() + .map(|&p| unsafe { CStr::from_ptr(p) }) + .map(|s| { + s.to_str().ok().and_then(|s| { + s.parse() + .map_err(|e| { + error!("Invalid -metricsallowip argument '{}': {}", s, e); + }) + .ok() + }) + }) + .collect() + { + Some(ips) => ips, + None => { return false; } }; - listen_address - .parse::() - .map_err(|e| { - error!( - "Invalid Prometheus metrics address '{}': {}", - listen_address, e - ); - () - }) - .and_then(|addr| { - PrometheusBuilder::new() - .listen_address(addr) - .install() - .map_err(|e| { - error!("Failed to start Prometheus metrics exporter: {:?}", e); - () - }) - }) - .is_ok() + // We always allow localhost. + allow_ips.extend(&["127.0.0.0/8".parse().unwrap(), "::1/128".parse().unwrap()]); + + // Parse the address to bind to. + let bind_address = SocketAddr::new( + if allow_ips.is_empty() { + // Default to loopback if not allowing external IPs. + "127.0.0.1".parse::().unwrap() + } else if bind_address.is_null() { + // No specific bind address specified, bind to any. + "0.0.0.0".parse::().unwrap() + } else { + match unsafe { CStr::from_ptr(bind_address) } + .to_str() + .ok() + .and_then(|s| s.parse::().ok()) + { + Some(addr) => addr, + None => { + error!("Invalid -metricsbind argument"); + return false; + } + } + }, + prometheus_port, + ); + + prometheus::install(bind_address, PrometheusBuilder::new(), allow_ips).is_ok() } pub struct FfiCallsite { diff --git a/src/rust/src/metrics_ffi/prometheus.rs b/src/rust/src/metrics_ffi/prometheus.rs new file mode 100644 index 000000000..2e7ddd6d3 --- /dev/null +++ b/src/rust/src/metrics_ffi/prometheus.rs @@ -0,0 +1,113 @@ +// This is mostly code copied from metrics_exporter_prometheus. It is licensed under the +// same license as the zcash codebase. + +use hyper::{ + server::{conn::AddrStream, Server}, + service::{make_service_fn, service_fn}, + {Body, Error as HyperError, Response, StatusCode}, +}; +use metrics::SetRecorderError; +use metrics_exporter_prometheus::{PrometheusBuilder, PrometheusRecorder}; +use std::future::Future; +use std::io; +use std::net::SocketAddr; +use std::thread; +use thiserror::Error as ThisError; +use tokio::{pin, runtime, select}; + +/// Errors that could occur while installing a Prometheus recorder/exporter. +#[derive(Debug, ThisError)] +pub enum InstallError { + /// Creating the networking event loop did not succeed. + #[error("failed to spawn Tokio runtime for endpoint: {0}")] + Io(#[from] io::Error), + + /// Binding/listening to the given address did not succeed. + #[error("failed to bind to given listen address: {0}")] + Hyper(#[from] HyperError), + + /// Installing the recorder did not succeed. + #[error("failed to install exporter as global recorder: {0}")] + Recorder(#[from] SetRecorderError), +} + +/// A copy of `PrometheusBuilder::build_with_exporter` that adds support for an IP address +/// or subnet allowlist. +pub(super) fn build( + bind_address: SocketAddr, + builder: PrometheusBuilder, + allow_ips: Vec, +) -> Result< + ( + PrometheusRecorder, + impl Future> + Send + 'static, + ), + InstallError, +> { + let recorder = builder.build(); + let handle = recorder.handle(); + + let server = Server::try_bind(&bind_address)?; + + let exporter = async move { + let make_svc = make_service_fn(move |socket: &AddrStream| { + let remote_addr = socket.remote_addr().ip(); + let allowed = allow_ips.iter().any(|subnet| subnet.contains(&remote_addr)); + let handle = handle.clone(); + + async move { + Ok::<_, HyperError>(service_fn(move |_| { + let handle = handle.clone(); + + async move { + if allowed { + let output = handle.render(); + Ok(Response::new(Body::from(output))) + } else { + Response::builder() + .status(StatusCode::FORBIDDEN) + .body(Body::empty()) + } + } + })) + } + }); + + server.serve(make_svc).await + }; + + Ok((recorder, exporter)) +} + +/// A copy of `PrometheusBuilder::install` that adds support for an IP address or subnet +/// allowlist. +pub(super) fn install( + bind_address: SocketAddr, + builder: PrometheusBuilder, + allow_ips: Vec, +) -> Result<(), InstallError> { + let runtime = runtime::Builder::new_current_thread() + .enable_all() + .build()?; + + let (recorder, exporter) = { + let _guard = runtime.enter(); + build(bind_address, builder, allow_ips)? + }; + metrics::set_boxed_recorder(Box::new(recorder))?; + + thread::Builder::new() + .name("zcash-prometheus".to_string()) + .spawn(move || { + runtime.block_on(async move { + pin!(exporter); + loop { + select! { + _ = &mut exporter => {} + } + } + }); + })?; + + Ok(()) +} diff --git a/src/util.cpp b/src/util.cpp index 1342c00f5..f7c6233c6 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -389,6 +389,7 @@ void ReadConfigFile(const std::string& confPath, "externalip", "fundingstream", "loadblock", + "metricsallowip", "nuparams", "onlynet", "rpcallowip", From c9e3d033203890f9e71e3a6fc98132da31766e4e Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 26 Feb 2021 00:57:31 +0000 Subject: [PATCH 17/27] rust: Pin hyper 0.14.2 hyper 0.14.3 added an unstable C API, but the changes to enable it require us to configure cargo with a linker for cross-compilation. We'll need to figure this out eventually, but for now let's just pin hyper to a version that doesn't require it. --- Cargo.lock | 9 +++++---- Cargo.toml | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 766216186..310387218 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -636,9 +636,9 @@ checksum = "494b4d60369511e7dea41cf646832512a94e542f68bb9c49e54518e0f468eb47" [[package]] name = "hyper" -version = "0.14.5" +version = "0.14.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8bf09f61b52cfcf4c00de50df88ae423d6c02354e385a86341133b5338630ad1" +checksum = "12219dc884514cb4a6a03737f4413c0e01c23a1b059b0156004b23f1e19dccbe" dependencies = [ "bytes", "futures-channel", @@ -1255,10 +1255,11 @@ checksum = "fe0f37c9e8f3c5a4a66ad655a93c74daac4ad00c441533bf5c6e7990bb42604e" [[package]] name = "socket2" -version = "0.4.0" +version = "0.3.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9e3dfc207c526015c632472a77be09cf1b6e46866581aecae5cc38fb4235dea2" +checksum = "122e570113d28d773067fab24266b66753f6ea915758651696b6e35e49f88d6e" dependencies = [ + "cfg-if 1.0.0", "libc", "winapi", ] diff --git a/Cargo.toml b/Cargo.toml index 86c225877..bf7362894 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,7 +38,7 @@ zcash_proofs = "0.5" ed25519-zebra = "2.0.0" # Metrics -hyper = { version = "0.14", default-features = false, features = ["server", "tcp", "http1"] } +hyper = { version = "=0.14.2", default-features = false, features = ["server", "tcp", "http1"] } ipnet = "2" metrics = "0.14.2" metrics-exporter-prometheus = "0.3" From 958ffeafd37c4af8d4c8e51177235e4c2504375c Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sun, 14 Mar 2021 08:08:31 +1300 Subject: [PATCH 18/27] metrics: Move documentation into zcashd book --- doc/book/src/SUMMARY.md | 2 ++ doc/book/src/user.md | 6 ++++++ contrib/metrics/README.md => doc/book/src/user/metrics.md | 0 3 files changed, 8 insertions(+) create mode 100644 doc/book/src/user.md rename contrib/metrics/README.md => doc/book/src/user/metrics.md (100%) diff --git a/doc/book/src/SUMMARY.md b/doc/book/src/SUMMARY.md index 0bca7e63c..65b038915 100644 --- a/doc/book/src/SUMMARY.md +++ b/doc/book/src/SUMMARY.md @@ -1,6 +1,8 @@ # The zcashd Book [zcashd](README.md) +- [User Documentation](user.md) + - [Metrics](user/metrics.md) - [Design](design.md) - [Chain state](design/chain-state.md) - ["Coins" view](design/coins-view.md) diff --git a/doc/book/src/user.md b/doc/book/src/user.md new file mode 100644 index 000000000..e23cdcf42 --- /dev/null +++ b/doc/book/src/user.md @@ -0,0 +1,6 @@ +# User Documentation + +This section contains user documentation specific to `zcashd`. + +See [here](https://zcash.readthedocs.io/) for more general Zcash documentation, as well as +installation instructions for `zcashd`. diff --git a/contrib/metrics/README.md b/doc/book/src/user/metrics.md similarity index 100% rename from contrib/metrics/README.md rename to doc/book/src/user/metrics.md From 78b83fd6e9330d2b986aad17ed4fd793dd07189e Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sun, 14 Mar 2021 11:25:27 +1300 Subject: [PATCH 19/27] metrics: Enable gauges with fully-static labels --- src/rust/include/rust/metrics.h | 72 +++++++++++++++++++++++++-------- src/rust/src/metrics_ffi.rs | 27 ++++++++++++- 2 files changed, 81 insertions(+), 18 deletions(-) diff --git a/src/rust/include/rust/metrics.h b/src/rust/include/rust/metrics.h index 8c8c53c90..c3133981e 100644 --- a/src/rust/include/rust/metrics.h +++ b/src/rust/include/rust/metrics.h @@ -34,13 +34,20 @@ typedef struct MetricsKey MetricsKey; /// Creates a metrics callsite. /// +/// This API supports labels that MUST have static values. For non-static label +/// values, use `metrics_key`. +/// /// You should usually call one of the helper macros such as `MetricsCounter` /// instead of calling this directly. /// /// This MUST ONLY be called to assign a `static MetricsCallsite*`, and all /// string arguments MUST be static `const char*` constants, and MUST be valid /// UTF-8. -MetricsCallsite* metrics_callsite(const char* name); +MetricsCallsite* metrics_callsite( + const char* name, + const char* const* label_names, + const char* const* label_values, + size_t labels_len); /// Creates a metrics key. /// @@ -133,20 +140,26 @@ void metrics_record_histogram(MetricsKey* callsite, double value); #ifdef __cplusplus // Constructs a metrics callsite. // -// The 'static constexpr' hack ensures that name is a compile-time constant -// with static storage duration. The output of this macro MUST be stored as a -// static MetricsCallsite*. -#define M_CALLSITE(name) ([&] { \ - static constexpr const char* _m_name = name; \ - return metrics_callsite(_m_name); \ +// The 'static constexpr' hack ensures that all arguments are compile-time +// constants with static storage duration. The output of this macro MUST be +// stored as a static MetricsCallsite*. +#define M_CALLSITE(name, label_names, label_values) ([&] { \ + static constexpr const char* _m_name = name; \ + static constexpr const char* const* _m_label_names = \ + label_names; \ + static constexpr const char* const* _m_label_values = \ + label_values; \ + return metrics_callsite( \ + _m_name, _m_label_names, _m_label_values, \ + T_ARRLEN(label_names)); \ }()) #else // Constructs a metrics callsite. // -// name MUST be a static constant, and the output of this macro MUST be stored -// as a static MetricsCallsite*. -#define M_CALLSITE(name) \ - metrics_callsite(name) +// All arguments MUST be static constants, and the output of this macro MUST be +// stored as a static MetricsCallsite*. +#define M_CALLSITE(name, label_names, label_values) \ + metrics_callsite(name, label_names, label_values, T_ARRLEN(label_names)) #endif // Constructs a metrics key. @@ -171,7 +184,9 @@ void metrics_record_histogram(MetricsKey* callsite, double value); #define MetricsCounter(name, value, ...) \ do { \ IFE(__VA_ARGS__) \ - (static MetricsCallsite* CALLSITE = M_CALLSITE(name); \ + (static constexpr const char* const EMPTY[] = {}; \ + static MetricsCallsite* CALLSITE = \ + M_CALLSITE(name, EMPTY, EMPTY); \ metrics_static_increment_counter(CALLSITE, value);) \ IFN(__VA_ARGS__)(const char* M_LABELS[] = \ {T_FIELD_NAMES(__VA_ARGS__)}; \ @@ -200,7 +215,9 @@ void metrics_record_histogram(MetricsKey* callsite, double value); #define MetricsGauge(name, value, ...) \ do { \ IFE(__VA_ARGS__) \ - (static MetricsCallsite* CALLSITE = M_CALLSITE(name); \ + (static constexpr const char* const EMPTY[] = {}; \ + static MetricsCallsite* CALLSITE = \ + M_CALLSITE(name, EMPTY, EMPTY); \ metrics_static_update_gauge(CALLSITE, value);) \ IFN(__VA_ARGS__)(const char* M_LABELS[] = \ {T_FIELD_NAMES(__VA_ARGS__)}; \ @@ -211,6 +228,23 @@ void metrics_record_histogram(MetricsKey* callsite, double value); metrics_update_gauge(KEY, value);) \ } while (0) +/// Updates a gauge with optional static labels. +/// +/// Gauges represent a single value that can go up or down over time, and always +/// starts out with an initial value of zero. +/// +/// name MUST be a static constant, and all strings MUST be valid UTF-8. +#define MetricsStaticGauge(name, value, ...) \ + do { \ + static constexpr const char* M_LABELS[] = \ + {T_FIELD_NAMES(__VA_ARGS__)}; \ + static constexpr const char* M_VALUES[] = \ + {T_FIELD_VALUES(__VA_ARGS__)}; \ + static MetricsCallsite* CALLSITE = \ + M_CALLSITE(name, M_LABELS, M_VALUES); \ + metrics_static_update_gauge(CALLSITE, value); \ + } while (0) + /// Increments a gauge. /// /// Gauges represent a single value that can go up or down over time, and always @@ -220,7 +254,9 @@ void metrics_record_histogram(MetricsKey* callsite, double value); #define MetricsIncrementGauge(name, value, ...) \ do { \ IFE(__VA_ARGS__) \ - (static MetricsCallsite* CALLSITE = M_CALLSITE(name); \ + (static constexpr const char* const EMPTY[] = {}; \ + static MetricsCallsite* CALLSITE = \ + M_CALLSITE(name, EMPTY, EMPTY); \ metrics_static_increment_gauge(CALLSITE, value);) \ IFN(__VA_ARGS__)(const char* M_LABELS[] = \ {T_FIELD_NAMES(__VA_ARGS__)}; \ @@ -240,7 +276,9 @@ void metrics_record_histogram(MetricsKey* callsite, double value); #define MetricsDecrementGauge(name, value, ...) \ do { \ IFE(__VA_ARGS__) \ - (static MetricsCallsite* CALLSITE = M_CALLSITE(name); \ + (static constexpr const char* const EMPTY[] = {}; \ + static MetricsCallsite* CALLSITE = \ + M_CALLSITE(name, EMPTY, EMPTY); \ metrics_static_decrement_gauge(CALLSITE, value);) \ IFN(__VA_ARGS__)(const char* M_LABELS[] = \ {T_FIELD_NAMES(__VA_ARGS__)}; \ @@ -260,7 +298,9 @@ void metrics_record_histogram(MetricsKey* callsite, double value); #define MetricsHistogram(name, value, ...) \ do { \ IFE(__VA_ARGS__) \ - (static MetricsCallsite* CALLSITE = M_CALLSITE(name); \ + (static constexpr const char* const EMPTY[] = {}; \ + static MetricsCallsite* CALLSITE = \ + M_CALLSITE(name, EMPTY, EMPTY); \ metrics_static_record_histogram(CALLSITE, value);) \ IFN(__VA_ARGS__)(const char* M_LABELS[] = \ {T_FIELD_NAMES(__VA_ARGS__)}; \ diff --git a/src/rust/src/metrics_ffi.rs b/src/rust/src/metrics_ffi.rs index 1c215f9c0..6acb51347 100644 --- a/src/rust/src/metrics_ffi.rs +++ b/src/rust/src/metrics_ffi.rs @@ -72,10 +72,33 @@ pub struct FfiCallsite { } #[no_mangle] -pub extern "C" fn metrics_callsite(name: *const c_char) -> *mut FfiCallsite { +pub extern "C" fn metrics_callsite( + name: *const c_char, + label_names: *const *const c_char, + label_values: *const *const c_char, + labels_len: usize, +) -> *mut FfiCallsite { let name = unsafe { CStr::from_ptr(name) }.to_str().unwrap(); + let labels = unsafe { slice::from_raw_parts(label_names, labels_len) }; + let values = unsafe { slice::from_raw_parts(label_values, labels_len) }; + + let stringify = |s: &[_]| { + s.iter() + .map(|&p| unsafe { CStr::from_ptr(p) }) + .map(|cs| cs.to_string_lossy().into_owned()) + .collect::>() + }; + let labels = stringify(labels); + let values = stringify(values); + + let labels: Vec<_> = labels + .into_iter() + .zip(values.into_iter()) + .map(|(name, value)| Label::new(name, value)) + .collect(); + Box::into_raw(Box::new(FfiCallsite { - key_data: KeyData::from_name(name), + key_data: KeyData::from_parts(name, labels), })) } From 0f9e4b9472b9ce960104039d3153e7c75da0692d Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sun, 14 Mar 2021 11:33:15 +1300 Subject: [PATCH 20/27] metrics: Use labels for pool statistics It's very likely that you'll want to operate over common pool statistics together. --- src/main.cpp | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index cf4bb62a4..00cbd2316 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3217,7 +3217,7 @@ void PruneAndFlush() { } struct PoolMetrics { - size_t commitments; + std::optional commitments; std::optional value; static PoolMetrics Sprout(CBlockIndex *pindex, CCoinsViewCache *view) { @@ -3239,22 +3239,28 @@ struct PoolMetrics { SaplingMerkleTree saplingTree; if (view->GetSaplingAnchorAt(pindex->hashFinalSaplingRoot, saplingTree)) { stats.commitments = saplingTree.size(); + } else { + stats.commitments = 0; } return stats; } }; -#define RenderPoolMetrics(poolName, poolMetrics) \ - do { \ - static constexpr const char* poolCommitments = \ - "pool." poolName ".commitments"; \ - static constexpr const char* poolValue = \ - "pool." poolName ".value.zatoshis"; \ - MetricsGauge(poolCommitments, poolMetrics.commitments); \ - if (poolMetrics.value) { \ - MetricsGauge(poolValue, poolMetrics.value.value()); \ - } \ +#define RenderPoolMetrics(poolName, poolMetrics) \ + do { \ + if (poolMetrics.commitments) { \ + MetricsStaticGauge( \ + "pool.commitments", \ + poolMetrics.commitments.value(), \ + "name", poolName); \ + } \ + if (poolMetrics.value) { \ + MetricsStaticGauge( \ + "pool.value.zatoshis", \ + poolMetrics.value.value(), \ + "name", poolName); \ + } \ } while (0) /** Update chainActive and related internal data structures. */ From e2e5df28a97ca17f9d8b6e86414165571e8bedd0 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sun, 14 Mar 2021 11:58:31 +1300 Subject: [PATCH 21/27] metrics: Rename metrics with consistent naming scheme - Add zcash. prefix to common metrics. - Use .total suffix for accumulating counters instead of .count. - Group names where possible. - Shorten names where possible (and still clear). --- doc/book/src/user/metrics.md | 16 ++++++++-------- src/main.cpp | 20 ++++++++++---------- src/net.cpp | 14 +++++++------- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/doc/book/src/user/metrics.md b/doc/book/src/user/metrics.md index cfbb27c11..59b86fe78 100644 --- a/doc/book/src/user/metrics.md +++ b/doc/book/src/user/metrics.md @@ -36,20 +36,20 @@ restarting `zcashd` you can then test the endpoint by querying it: ``` $ curl http://127.0.0.1: -# TYPE peer_outbound_messages counter -peer_outbound_messages 181 +# TYPE zcash_net_out_messages counter +zcash_net_out_messages 181 -# TYPE bytes_read counter -bytes_read 3701998 +# TYPE zcash_net_in_bytes_total counter +zcash_net_in_bytes_total 3701998 -# TYPE peer_inbound_messages counter -peer_inbound_messages 184 +# TYPE zcash_net_in_messages counter +zcash_net_in_messages 184 # TYPE zcashd_build_info counter zcashd_build_info{version="v4.2.0"} 1 -# TYPE block_verified_block_count counter -block_verified_block_count 162 +# TYPE zcash_chain_verified_block_total counter +zcash_chain_verified_block_total 162 ... ``` diff --git a/src/main.cpp b/src/main.cpp index 00cbd2316..a9bea3f58 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1694,9 +1694,9 @@ bool AcceptToMemoryPool( pool.EnsureSizeLimit(); - MetricsGauge("mempool.size.transactions", mempool.size()); - MetricsGauge("mempool.size.bytes", mempool.GetTotalTxSize()); - MetricsGauge("mempool.usage.bytes", mempool.DynamicMemoryUsage()); + MetricsGauge("zcash.mempool.size.transactions", mempool.size()); + MetricsGauge("zcash.mempool.size.bytes", mempool.GetTotalTxSize()); + MetricsGauge("zcash.mempool.usage.bytes", mempool.DynamicMemoryUsage()); } } @@ -3251,13 +3251,13 @@ struct PoolMetrics { do { \ if (poolMetrics.commitments) { \ MetricsStaticGauge( \ - "pool.commitments", \ + "zcash.pool.commitments", \ poolMetrics.commitments.value(), \ "name", poolName); \ } \ if (poolMetrics.value) { \ MetricsStaticGauge( \ - "pool.value.zatoshis", \ + "zcash.pool.value.zatoshis", \ poolMetrics.value.value(), \ "name", poolName); \ } \ @@ -3293,7 +3293,7 @@ void static UpdateTip(CBlockIndex *pindexNew, const CChainParams& chainParams) { auto sproutPool = PoolMetrics::Sprout(pindexNew, pcoinsTip); auto saplingPool = PoolMetrics::Sapling(pindexNew, pcoinsTip); - MetricsGauge("block.verified.block.height", pindexNew->nHeight); + MetricsGauge("zcash.chain.verified.block.height", pindexNew->nHeight); RenderPoolMetrics("sprout", sproutPool); RenderPoolMetrics("sapling", saplingPool); @@ -3428,8 +3428,8 @@ bool static ConnectTip(CValidationState& state, const CChainParams& chainparams, int64_t nTime6 = GetTimeMicros(); nTimePostConnect += nTime6 - nTime5; nTimeTotal += nTime6 - nTime1; LogPrint("bench", " - Connect postprocess: %.2fms [%.2fs]\n", (nTime6 - nTime5) * 0.001, nTimePostConnect * 0.000001); LogPrint("bench", "- Connect block: %.2fms [%.2fs]\n", (nTime6 - nTime1) * 0.001, nTimeTotal * 0.000001); - MetricsIncrementCounter("block.verified.block.count"); - MetricsHistogram("block.verified.block.seconds", (nTime6 - nTime1) * 0.000001); + MetricsIncrementCounter("zcash.chain.verified.block.total"); + MetricsHistogram("zcash.chain.verified.block.seconds", (nTime6 - nTime1) * 0.000001); return true; } @@ -6287,7 +6287,7 @@ bool static ProcessMessage(const CChainParams& chainparams, CNode* pfrom, string CInv inv(MSG_BLOCK, block.GetHash()); LogPrint("net", "received block %s peer=%d\n", inv.hash.ToString(), pfrom->id); - MetricsIncrementCounter("sync.downloaded.block.count"); + MetricsIncrementCounter("zcash.sync.block.downloaded.total"); pfrom->AddInventoryKnown(inv); @@ -6307,7 +6307,7 @@ bool static ProcessMessage(const CChainParams& chainparams, CNode* pfrom, string Misbehaving(pfrom->GetId(), nDoS); } } else if (state.IsValid()) { - MetricsIncrementCounter("sync.verified.block.count"); + MetricsIncrementCounter("zcash.sync.block.verified.total"); } } diff --git a/src/net.cpp b/src/net.cpp index ea0014b57..d37703ad6 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -705,9 +705,9 @@ bool CNode::ReceiveMsgBytes(const char *pch, unsigned int nBytes) if (msg.complete()) { msg.nTime = GetTimeMicros(); std::string strCommand = SanitizeString(msg.hdr.GetCommand()); - MetricsIncrementCounter("peer.inbound.messages", "command", strCommand.c_str()); + MetricsIncrementCounter("zcash.net.in.messages", "command", strCommand.c_str()); MetricsCounter( - "peer.inbound.bytes", msg.hdr.nMessageSize, + "zcash.net.in.bytes", msg.hdr.nMessageSize, "command", strCommand.c_str()); messageHandlerCondition.notify_one(); } @@ -1111,7 +1111,7 @@ void ThreadSocketHandler() } if (vNodesSize != nPrevNodeCount) { nPrevNodeCount = vNodesSize; - MetricsGauge("pool.num_peers", nPrevNodeCount); + MetricsGauge("zcash.net.peers", nPrevNodeCount); uiInterface.NotifyNumConnectionsChanged(nPrevNodeCount); } @@ -2009,14 +2009,14 @@ void CNode::RecordBytesRecv(uint64_t bytes) { LOCK(cs_totalBytesRecv); nTotalBytesRecv += bytes; - MetricsCounter("bytes.read", bytes); + MetricsCounter("zcash.net.in.bytes.total", bytes); } void CNode::RecordBytesSent(uint64_t bytes) { LOCK(cs_totalBytesSent); nTotalBytesSent += bytes; - MetricsCounter("bytes.written", bytes); + MetricsCounter("zcash.net.out.bytes.total", bytes); uint64_t now = GetTime(); if (nMaxOutboundCycleStartTime + nMaxOutboundTimeframe < now) @@ -2292,7 +2292,7 @@ void CNode::AbortMessage() UNLOCK_FUNCTION(cs_vSend) void CNode::EndMessage() UNLOCK_FUNCTION(cs_vSend) { - MetricsIncrementCounter("peer.outbound.messages", "command", strSendCommand.c_str()); + MetricsIncrementCounter("zcash.net.out.messages", "command", strSendCommand.c_str()); // The -*messagestest options are intentionally not documented in the help message, // since they are only used during development to debug the networking code and are // not intended for end-users. @@ -2327,7 +2327,7 @@ void CNode::EndMessage() UNLOCK_FUNCTION(cs_vSend) ssSend.GetAndClear(*it); nSendSize += (*it).size(); MetricsCounter( - "peer.outbound.bytes", (*it).size(), + "zcash.net.out.bytes", (*it).size(), "command", strSendCommand.c_str()); strSendCommand.clear(); From 0cab2e70940ed4c39ab46b6a58b64cd8faf56a5e Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sun, 14 Mar 2021 12:00:37 +1300 Subject: [PATCH 22/27] metrics: Remove zcash.sync.* metrics - zcash.sync.block.downloaded.total is redundant, and can be replaced by zcash.net.in.messages[command=block]. - zcash.sync.block.verified.total and zcash.chain.verified.block.total are identical for us, because we verify blocks synchronously. --- src/main.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index a9bea3f58..a8d0d360c 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -6287,7 +6287,6 @@ bool static ProcessMessage(const CChainParams& chainparams, CNode* pfrom, string CInv inv(MSG_BLOCK, block.GetHash()); LogPrint("net", "received block %s peer=%d\n", inv.hash.ToString(), pfrom->id); - MetricsIncrementCounter("zcash.sync.block.downloaded.total"); pfrom->AddInventoryKnown(inv); @@ -6306,8 +6305,6 @@ bool static ProcessMessage(const CChainParams& chainparams, CNode* pfrom, string LOCK(cs_main); Misbehaving(pfrom->GetId(), nDoS); } - } else if (state.IsValid()) { - MetricsIncrementCounter("zcash.sync.block.verified.total"); } } From f8d63a83fc4b649bf22ee9bd93103fc562d807bf Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Sun, 14 Mar 2021 12:55:31 +1300 Subject: [PATCH 23/27] metrics: Rework pool metrics in anticipation of transparent pool notes.created and notes.spent can be collected for the transparent pool as well as the shielded pools. notes.unspent can only be collected for the transparent pool, by design. --- src/main.cpp | 42 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index a8d0d360c..2f19c88d6 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3217,7 +3217,9 @@ void PruneAndFlush() { } struct PoolMetrics { - std::optional commitments; + std::optional created; + std::optional spent; + std::optional unspent; std::optional value; static PoolMetrics Sprout(CBlockIndex *pindex, CCoinsViewCache *view) { @@ -3226,7 +3228,7 @@ struct PoolMetrics { SproutMerkleTree sproutTree; assert(view->GetSproutAnchorAt(pindex->hashFinalSproutRoot, sproutTree)); - stats.commitments = sproutTree.size(); + stats.created = sproutTree.size(); return stats; } @@ -3235,24 +3237,46 @@ struct PoolMetrics { PoolMetrics stats; stats.value = pindex->nChainSaplingValue; - // Before Sapling activation, stats.commitments will be zero. + // Before Sapling activation, the Sapling commitment set is empty. SaplingMerkleTree saplingTree; if (view->GetSaplingAnchorAt(pindex->hashFinalSaplingRoot, saplingTree)) { - stats.commitments = saplingTree.size(); + stats.created = saplingTree.size(); } else { - stats.commitments = 0; + stats.created = 0; } return stats; } + + static PoolMetrics Transparent(CBlockIndex *pindex, CCoinsViewCache *view) { + PoolMetrics stats; + // TODO: Collect transparent pool value. + + // TODO: Figure out a way to efficiently collect UTXO set metrics + // (view->GetStats() is too slow to call during block verification). + + return stats; + } }; #define RenderPoolMetrics(poolName, poolMetrics) \ do { \ - if (poolMetrics.commitments) { \ + if (poolMetrics.created) { \ MetricsStaticGauge( \ - "zcash.pool.commitments", \ - poolMetrics.commitments.value(), \ + "zcash.pool.notes.created", \ + poolMetrics.created.value(), \ + "name", poolName); \ + } \ + if (poolMetrics.spent) { \ + MetricsStaticGauge( \ + "zcash.pool.notes.spent", \ + poolMetrics.spent.value(), \ + "name", poolName); \ + } \ + if (poolMetrics.unspent) { \ + MetricsStaticGauge( \ + "zcash.pool.notes.unspent", \ + poolMetrics.unspent.value(), \ "name", poolName); \ } \ if (poolMetrics.value) { \ @@ -3292,10 +3316,12 @@ void static UpdateTip(CBlockIndex *pindexNew, const CChainParams& chainParams) { auto sproutPool = PoolMetrics::Sprout(pindexNew, pcoinsTip); auto saplingPool = PoolMetrics::Sapling(pindexNew, pcoinsTip); + auto transparentPool = PoolMetrics::Transparent(pindexNew, pcoinsTip); MetricsGauge("zcash.chain.verified.block.height", pindexNew->nHeight); RenderPoolMetrics("sprout", sproutPool); RenderPoolMetrics("sapling", saplingPool); + RenderPoolMetrics("transparent", transparentPool); cvBlockChange.notify_all(); } From 3030df906d7125d2751109b6b3d96e935bfcb980 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 31 Mar 2021 09:08:55 +1300 Subject: [PATCH 24/27] net: Clear CNode::strSendCommand if a message is aborted --- src/net.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/net.cpp b/src/net.cpp index d37703ad6..9026140ca 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2284,6 +2284,7 @@ void CNode::BeginMessage(const char* pszCommand) EXCLUSIVE_LOCK_FUNCTION(cs_vSen void CNode::AbortMessage() UNLOCK_FUNCTION(cs_vSend) { ssSend.clear(); + strSendCommand.clear(); LEAVE_CRITICAL_SECTION(cs_vSend); From 83eef40f4c021fcb53bc969af534b49683d29ed6 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 31 Mar 2021 09:15:23 +1300 Subject: [PATCH 25/27] rust: Add license header to metrics_ffi::prometheus --- contrib/debian/copyright | 6 ++++++ src/rust/src/metrics_ffi/prometheus.rs | 15 +++++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/contrib/debian/copyright b/contrib/debian/copyright index d2c2fbc58..8b048579e 100644 --- a/contrib/debian/copyright +++ b/contrib/debian/copyright @@ -137,6 +137,12 @@ Files: src/rust/include/tracing.h Copyright: Copyright (c) 2020 Jack Grigg License: Expat +Files: src/rust/src/metrics_ffi/prometheus.rs +Copyright: + 2020-2021 The contributors to the metrics project + 2021 Jack Grigg +License: Expat + Files: src/secp256k1/* Copyright: Copyright (c) 2013 Pieter Wuille License: Expat diff --git a/src/rust/src/metrics_ffi/prometheus.rs b/src/rust/src/metrics_ffi/prometheus.rs index 2e7ddd6d3..da11f5f6b 100644 --- a/src/rust/src/metrics_ffi/prometheus.rs +++ b/src/rust/src/metrics_ffi/prometheus.rs @@ -1,5 +1,16 @@ -// This is mostly code copied from metrics_exporter_prometheus. It is licensed under the -// same license as the zcash codebase. +// This is mostly code copied from metrics_exporter_prometheus. The copied portions are +// licensed under the same terms as the zcash codebase (reproduced below from +// https://github.com/metrics-rs/metrics/blob/main/metrics-exporter-prometheus/LICENSE): +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. use hyper::{ server::{conn::AddrStream, Server}, From c940fd302b1955e240333004b0634822d2a59bd3 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 31 Mar 2021 09:21:16 +1300 Subject: [PATCH 26/27] book: Fix typo in metrics documentation --- doc/book/src/user/metrics.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/book/src/user/metrics.md b/doc/book/src/user/metrics.md index 59b86fe78..dd11d6d95 100644 --- a/doc/book/src/user/metrics.md +++ b/doc/book/src/user/metrics.md @@ -30,7 +30,7 @@ You can see what each method provides with `zcash-cli help METHOD_NAME`. `zcashd` can optionally expose an HTTP server that acts as a Prometheus scrape endpoint. The server will respond to `GET` requests on any request path. -To enable the endpoint, add `-prometheusport=:` to your `zcashd` +To enable the endpoint, add `-prometheusport=` to your `zcashd` configuration (either in `zcash.conf` or on the command line). After restarting `zcashd` you can then test the endpoint by querying it: From b845868e2af509ca94c68b29e76d14b5e710474c Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 31 Mar 2021 12:45:38 +1300 Subject: [PATCH 27/27] metrics: Don't assert that the Sprout tree is accessible RewindBlockIndex calls DisconnectTip in a way that can potentially cause a Sprout tree to not exist (the rewind_index RPC test reliably triggers this). We only need to access the tree during disconnection for metrics purposes, and we will never encounter this rewind situation on either mainnet or testnet, so if we can't access the Sprout tree we default to zero. --- src/main.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 2f19c88d6..ed774e5fd 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3226,9 +3226,17 @@ struct PoolMetrics { PoolMetrics stats; stats.value = pindex->nChainSproutValue; + // RewindBlockIndex calls DisconnectTip in a way that can potentially cause a + // Sprout tree to not exist (the rewind_index RPC test reliably triggers this). + // We only need to access the tree during disconnection for metrics purposes, and + // we will never encounter this rewind situation on either mainnet or testnet, so + // if we can't access the Sprout tree we default to zero. SproutMerkleTree sproutTree; - assert(view->GetSproutAnchorAt(pindex->hashFinalSproutRoot, sproutTree)); - stats.created = sproutTree.size(); + if (view->GetSproutAnchorAt(pindex->hashFinalSproutRoot, sproutTree)) { + stats.created = sproutTree.size(); + } else { + stats.created = 0; + } return stats; }