[feature] #3964: application/x-parity-scale response for /status … (

#3983)

Signed-off-by: Dmitry Balashov <a.marcius26@gmail.com>
Signed-off-by: 6r1d <vic.6r1d@gmail.com>

Cargo.toml

@@ -100,6 +100,7 @@ duct = "0.13.6"
 
 criterion = "0.5.1"
 proptest = "1.3.1"
+expect-test = "1.4.1"
 
 eyre = "0.6.8"
 color-eyre = "0.6.2"

cli/src/torii/mod.rs

@@ -58,6 +58,10 @@ pub enum Error {
     #[cfg(feature = "telemetry")]
     /// Error while getting Prometheus metrics
     Prometheus(#[source] eyre::Report),
+    /// Internal error while getting status
+    StatusFailure(#[source] eyre::Report),
+    /// Cannot find status segment by provided path
+    StatusSegmentNotFound(#[source] eyre::Report),
 }
 
 impl Reply for Error {
@@ -79,14 +83,14 @@ impl Error {
         match self {
             Query(e) => Self::query_status_code(e),
             AcceptTransaction(_) | ConfigurationReload(_) => StatusCode::BAD_REQUEST,
-            Config(_) => StatusCode::NOT_FOUND,
+            Config(_) | StatusSegmentNotFound(_) => StatusCode::NOT_FOUND,
             PushIntoQueue(err) => match **err {
                 queue::Error::Full => StatusCode::INTERNAL_SERVER_ERROR,
                 queue::Error::SignatureCondition { .. } => StatusCode::UNAUTHORIZED,
                 _ => StatusCode::BAD_REQUEST,
             },
             #[cfg(feature = "telemetry")]
-            Prometheus(_) => StatusCode::INTERNAL_SERVER_ERROR,
+            Prometheus(_) | StatusFailure(_) => StatusCode::INTERNAL_SERVER_ERROR,
         }
     }
 

cli/src/torii/routing.rs

@@ -7,7 +7,7 @@
 
 use std::num::NonZeroUsize;
 
-use eyre::WrapErr;
+use eyre::{eyre, WrapErr};
 use futures::TryStreamExt;
 use iroha_config::{
     base::proxy::Documented,
@@ -349,36 +349,49 @@ fn handle_metrics(sumeragi: &SumeragiHandle) -> Result<String> {
         .map_err(Error::Prometheus)
 }
 
-#[cfg(feature = "telemetry")]
-#[allow(clippy::unnecessary_wraps)]
-fn handle_status(sumeragi: &SumeragiHandle) -> Result<warp::reply::Json, Infallible> {
+fn update_metrics_gracefully(sumeragi: &SumeragiHandle) {
     if let Err(error) = sumeragi.update_metrics() {
         iroha_logger::error!(%error, "Error while calling `sumeragi::update_metrics`.");
     }
-    let status = Status::from(&sumeragi.metrics());
-    Ok(reply::json(&status))
 }
 
 #[cfg(feature = "telemetry")]
-#[allow(clippy::unused_async)]
-async fn handle_status_precise(sumeragi: SumeragiHandle, segment: String) -> Result<Json> {
-    if let Err(error) = sumeragi.update_metrics() {
-        iroha_logger::error!(%error, "Error while calling `sumeragi::update_metrics`.");
-    }
-    // TODO: This probably can be optimised to elide the full
-    // structure. Ideally there should remain a list of fields and
-    // field aliases somewhere in `serde` macro output, which can
-    // elide the creation of the value, and directly read the value
-    // behind the mutex.
+#[allow(clippy::unnecessary_wraps)]
+fn handle_status(
+    sumeragi: &SumeragiHandle,
+    accept: Option<impl AsRef<str>>,
+    tail: &warp::path::Tail,
+) -> Result<Response> {
+    use eyre::ContextCompat;
+
+    update_metrics_gracefully(sumeragi);
     let status = Status::from(&sumeragi.metrics());
-    match serde_json::to_value(status) {
-        Ok(value) => Ok(value
-            .get(segment)
-            .map_or_else(|| reply::json(&value), reply::json)),
-        Err(err) => {
-            iroha_logger::error!(%err, "Error while converting to JSON value");
-            Ok(reply::json(&None::<String>))
+
+    let tail = tail.as_str();
+    if tail.is_empty() {
+        if accept.is_some_and(|x| x.as_ref() == PARITY_SCALE_MIME_TYPE) {
+            Ok(Scale(status).into_response())
+        } else {
+            Ok(reply::json(&status).into_response())
         }
+    } else {
+        // TODO: This probably can be optimised to elide the full
+        // structure. Ideally there should remain a list of fields and
+        // field aliases somewhere in `serde` macro output, which can
+        // elide the creation of the value, and directly read the value
+        // behind the mutex.
+        let value = serde_json::to_value(status)
+            .wrap_err("Failed to serialize JSON")
+            .map_err(Error::StatusFailure)?;
+
+        let reply = tail
+            .split('/')
+            .try_fold(&value, serde_json::Value::get)
+            .wrap_err_with(|| eyre!("Path not found: \"{}\"", tail))
+            .map_err(Error::StatusSegmentNotFound)
+            .map(|segment| reply::json(segment).into_response())?;
+
+        Ok(reply)
     }
 }
 
@@ -427,19 +440,13 @@ impl Torii {
             )),
         );
 
-        let status_path = warp::path(uri::STATUS);
-        let get_router_status_precise = endpoint2(
-            handle_status_precise,
-            status_path
-                .and(add_state!(self.sumeragi.clone()))
-                .and(warp::path::param()),
-        );
-        let get_router_status_bare =
-            status_path
-                .and(add_state!(self.sumeragi.clone()))
-                .and_then(|sumeragi| async move {
-                    Ok::<_, Infallible>(WarpResult(handle_status(&sumeragi)))
-                });
+        let get_router_status = warp::path(uri::STATUS)
+            .and(add_state!(self.sumeragi.clone()))
+            .and(warp::header::optional(warp::http::header::ACCEPT.as_str()))
+            .and(warp::path::tail())
+            .and_then(|sumeragi, accept: Option<String>, tail| async move {
+                Ok::<_, Infallible>(WarpResult(handle_status(&sumeragi, accept.as_ref(), &tail)))
+            });
         let get_router_metrics = warp::path(uri::METRICS)
             .and(add_state!(self.sumeragi))
             .and_then(|sumeragi| async move {
@@ -451,7 +458,7 @@ impl Torii {
 
         #[cfg(feature = "telemetry")]
         let get_router = get_router.or(warp::any()
-            .and(get_router_status_precise.or(get_router_status_bare))
+            .and(get_router_status)
             .or(get_router_metrics)
             .or(get_api_version));
 

cli/src/torii/utils.rs

@@ -1,7 +1,12 @@
 use std::convert::Infallible;
 
 use iroha_version::prelude::*;
-use warp::{hyper::body::Bytes, reply::Response, Filter, Rejection, Reply};
+use warp::{
+    http::{header::CONTENT_TYPE, HeaderValue},
+    hyper::body::Bytes,
+    reply::Response,
+    Filter, Rejection, Reply,
+};
 
 /// Structure for empty response body
 #[derive(Clone, Copy)]
@@ -13,13 +18,22 @@ impl Reply for Empty {
     }
 }
 
-/// Structure for response in scale codec in body
+/// MIME used in Torii for SCALE encoding
+// note: no elegant way to associate it with generic `Scale<T>`
+pub const PARITY_SCALE_MIME_TYPE: &'_ str = "application/x-parity-scale";
+
+/// Structure to reply using SCALE encoding
 #[derive(Debug)]
 pub struct Scale<T>(pub T);
 
 impl<T: Encode + Send> Reply for Scale<T> {
     fn into_response(self) -> Response {
-        Response::new(self.0.encode().into())
+        let mut res = Response::new(self.0.encode().into());
+        res.headers_mut().insert(
+            CONTENT_TYPE,
+            HeaderValue::from_static(PARITY_SCALE_MIME_TYPE),
+        );
+        res
     }
 }
 

scripts/test_env.py

@@ -158,7 +158,7 @@ def copy_or_prompt_build_bin(bin_name: str, root_dir: pathlib.Path, target_dir:
                 logging.critical("Can't launch the network without the binary. Aborting...")
                 sys.exit(4)
             else:
-                logging.error("Please answer with either `y[es]` or `n[o])")
+                logging.error("Please answer with either `y[es]` or `n[o]`")
 
 def main(args: argparse.Namespace):
     # Bold ASCII escape sequence

telemetry/Cargo.toml

@@ -35,9 +35,13 @@ tokio-stream = { workspace = true, features = ["fs"] }
 tokio-tungstenite = { workspace = true }
 url = { workspace = true, features = ["serde"] }
 prometheus = { workspace = true }
-
+parity-scale-codec = { workspace = true }
 
 [build-dependencies]
 eyre = { workspace = true }
 vergen = { workspace = true, features = ["cargo", "git", "gitoxide"] }
 
+[dev-dependencies]
+expect-test = { workspace = true }
+hex = { workspace = true }
+

telemetry/src/metrics.rs

@@ -5,6 +5,7 @@ use std::{
     time::{Duration, SystemTime},
 };
 
+use parity_scale_codec::{Compact, Encode};
 use prometheus::{
     core::{AtomicU64, GenericGauge, GenericGaugeVec},
     Encoder, Histogram, HistogramOpts, HistogramVec, IntCounter, IntCounterVec, Opts, Registry,
@@ -21,22 +22,38 @@ impl Default for Uptime {
     }
 }
 
+impl Encode for Uptime {
+    fn encode(&self) -> Vec<u8> {
+        let secs = self.0.as_secs();
+        let nanos = self.0.subsec_nanos();
+        // While seconds are rarely very large, nanos could be anywhere between zero and one billion,
+        // eliminating the profit of Compact
+        (Compact(secs), nanos).encode()
+    }
+}
+
 /// Response body for GET status request
-#[derive(Clone, Copy, Debug, Default, Deserialize, Serialize)]
+#[derive(Clone, Copy, Debug, Default, Deserialize, Serialize, Encode)]
 pub struct Status {
     /// Number of connected peers, except for the reporting peer itself
+    #[codec(compact)]
     pub peers: u64,
     /// Number of committed blocks
+    #[codec(compact)]
     pub blocks: u64,
     /// Number of accepted transactions
+    #[codec(compact)]
     pub txs_accepted: u64,
     /// Number of rejected transactions
+    #[codec(compact)]
     pub txs_rejected: u64,
     /// Uptime since genesis block creation
     pub uptime: Uptime,
     /// Number of view changes in the current round
+    #[codec(compact)]
     pub view_changes: u64,
     /// Number of the transactions in the queue
+    #[codec(compact)]
     pub queue_size: u64,
 }
 
@@ -205,6 +222,8 @@ impl Metrics {
 
 #[cfg(test)]
 mod test {
+    #![allow(clippy::restriction)]
+
     use super::*;
 
     #[test]
@@ -219,4 +238,51 @@ mod test {
         println!("{:?}", Status::from(&Box::new(metrics)));
         println!("{:?}", Status::default());
     }
+
+    fn sample_status() -> Status {
+        Status {
+            peers: 4,
+            blocks: 5,
+            txs_accepted: 31,
+            txs_rejected: 3,
+            uptime: Uptime(Duration::new(5, 937_000_000)),
+            view_changes: 2,
+            queue_size: 18,
+        }
+    }
+
+    #[test]
+    fn serialize_status_json() {
+        let value = sample_status();
+
+        let actual = serde_json::to_string_pretty(&value).expect("Sample is valid");
+        // CAUTION: if this is outdated, make sure to update the documentation:
+        // https://hyperledger.github.io/iroha-2-docs/api/torii-endpoints#status
+        let expected = expect_test::expect![[r#"
+            {
+              "peers": 4,
+              "blocks": 5,
+              "txs_accepted": 31,
+              "txs_rejected": 3,
+              "uptime": {
+                "secs": 5,
+                "nanos": 937000000
+              },
+              "view_changes": 2,
+              "queue_size": 18
+            }"#]];
+        expected.assert_eq(&actual);
+    }
+
+    #[test]
+    fn serialize_status_scale() {
+        let value = sample_status();
+        let bytes = value.encode();
+
+        let actual = hex::encode_upper(bytes);
+        // CAUTION: if this is outdated, make sure to update the documentation:
+        // https://hyperledger.github.io/iroha-2-docs/api/torii-endpoints#status
+        let expected = expect_test::expect!["10147C0C14407CD9370848"];
+        expected.assert_eq(&actual);
+    }
 }

tools/swarm/Cargo.toml

@@ -27,4 +27,4 @@ inquire.workspace = true
 [dev-dependencies]
 iroha_config.workspace = true
 
-expect-test = "1.4.1"
+expect-test.workspace = true