tracing: Split HTML-formatting into admin module (#1025)

The tracing crate is fairly core to all of our functionality, and so
it's preferable to avoid pulling in HTML-specific dependencies here. In
order to keep the tracing crate limited to only tracing-related
functionality, this change moves the HTTP handlers to the
`app::core::admin`, which includes the rest of the HTTP serving logic
used by the admin server.

There is no functional change.

Author: olix0r

Cargo.lock

@@ -416,9 +416,9 @@ dependencies = [
 
 [[package]]
 name = "html-escape"
-version = "0.2.6"
+version = "0.2.9"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d348900ce941b7474395ba922ed3735a517df4546a2939ddb416ce85eeaa988e"
+checksum = "816ea801a95538fc5f53c836697b3f8b64a9d664c4f0b91efe1fe7c92e4dbcb7"
 dependencies = [
  "utf8-width",
 ]
@@ -639,6 +639,7 @@ version = "0.1.0"
 dependencies = [
  "bytes",
  "futures",
+ "html-escape",
  "http",
  "http-body",
  "hyper",
@@ -689,6 +690,7 @@ dependencies = [
  "pin-project",
  "prost-types",
  "regex",
+ "serde_json",
  "thiserror",
  "tokio",
  "tokio-stream",
@@ -1422,12 +1424,7 @@ dependencies = [
 name = "linkerd-tracing"
 version = "0.1.0"
 dependencies = [
- "bytes",
- "html-escape",
- "http",
- "hyper",
  "linkerd-error",
- "serde_json",
  "tokio",
  "tokio-trace",
  "tracing",
@@ -2487,9 +2484,9 @@ dependencies = [
 
 [[package]]
 name = "utf8-width"
-version = "0.1.4"
+version = "0.1.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "9071ac216321a4470a69fb2b28cfc68dcd1a39acd877c8be8e014df6772d8efa"
+checksum = "7cf7d77f457ef8dfa11e4cd5933c5ddb5dc52a94664071951219a97710f0a32b"
 
 [[package]]
 name = "want"

linkerd/addr/fuzz/Cargo.lock

@@ -168,15 +168,6 @@ dependencies = [
  "libc",
 ]
 
-[[package]]
-name = "html-escape"
-version = "0.2.9"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "816ea801a95538fc5f53c836697b3f8b64a9d664c4f0b91efe1fe7c92e4dbcb7"
-dependencies = [
- "utf8-width",
-]
-
 [[package]]
 name = "http"
 version = "0.2.4"
@@ -188,51 +179,6 @@ dependencies = [
  "itoa",
 ]
 
-[[package]]
-name = "http-body"
-version = "0.4.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "60daa14be0e0786db0f03a9e57cb404c9d756eed2b6c62b9ea98ec5743ec75a9"
-dependencies = [
- "bytes",
- "http",
- "pin-project-lite",
-]
-
-[[package]]
-name = "httparse"
-version = "1.4.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f3a87b616e37e93c22fb19bcd386f02f3af5ea98a25670ad0fce773de23c5e68"
-
-[[package]]
-name = "httpdate"
-version = "1.0.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "05842d0d43232b23ccb7060ecb0f0626922c21f30012e97b767b30afd4a5d4b9"
-
-[[package]]
-name = "hyper"
-version = "0.14.7"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1e5f105c494081baa3bf9e200b279e27ec1623895cd504c7dbef8d0b080fcf54"
-dependencies = [
- "bytes",
- "futures-channel",
- "futures-core",
- "futures-util",
- "http",
- "http-body",
- "httparse",
- "httpdate",
- "itoa",
- "pin-project",
- "tokio",
- "tower-service",
- "tracing",
- "want",
-]
-
 [[package]]
 name = "instant"
 version = "0.1.9"
@@ -318,12 +264,7 @@ dependencies = [
 name = "linkerd-tracing"
 version = "0.1.0"
 dependencies = [
- "bytes",
- "html-escape",
- "http",
- "hyper",
  "linkerd-error",
- "serde_json",
  "tokio",
  "tokio-trace",
  "tracing",
@@ -405,26 +346,6 @@ dependencies = [
  "winapi",
 ]
 
-[[package]]
-name = "pin-project"
-version = "1.0.7"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "c7509cc106041c40a4518d2af7a61530e1eed0e6285296a3d8c5472806ccc4a4"
-dependencies = [
- "pin-project-internal",
-]
-
-[[package]]
-name = "pin-project-internal"
-version = "1.0.7"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "48c950132583b500556b1efd71d45b319029f2b71518d979fcc208e16b42426f"
-dependencies = [
- "proc-macro2",
- "quote",
- "syn",
-]
-
 [[package]]
 name = "pin-project-lite"
 version = "0.2.6"
@@ -635,12 +556,6 @@ dependencies = [
  "tracing-subscriber",
 ]
 
-[[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"
@@ -716,12 +631,6 @@ dependencies = [
  "tracing-serde",
 ]
 
-[[package]]
-name = "try-lock"
-version = "0.2.3"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "59547bce71d9c38b83d9c0e92b6066c4253371f15005def0c30d9657f50c7642"
-
 [[package]]
 name = "unicode-xid"
 version = "0.2.1"
@@ -734,22 +643,6 @@ version = "0.7.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "a156c684c91ea7d62626509bce3cb4e1d9ed5c4d978f7b4352658f96a4c26b4a"
 
-[[package]]
-name = "utf8-width"
-version = "0.1.5"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7cf7d77f457ef8dfa11e4cd5933c5ddb5dc52a94664071951219a97710f0a32b"
-
-[[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 = "wasm-bindgen"
 version = "0.2.73"

linkerd/app/core/Cargo.toml

@@ -15,6 +15,7 @@ independently of the inbound and outbound proxy logic.
 [dependencies]
 bytes = "1"
 http = "0.2"
+html-escape = "0.2"
 http-body = "0.4"
 hyper = { version = "0.14.2", features = ["http1", "http2"] }
 futures = "0.3.15"
@@ -35,7 +36,6 @@ linkerd-error-respond = { path = "../../error-respond" }
 linkerd-exp-backoff = { path = "../../exp-backoff" }
 linkerd-http-classify = { path = "../../http-classify" }
 linkerd-http-metrics = { path = "../../http-metrics" }
-
 linkerd-http-retry = { path = "../../http-retry" }
 linkerd-identity = { path = "../../identity" }
 linkerd-io = { path = "../../io" }
@@ -63,6 +63,7 @@ linkerd-stack-tracing = { path = "../../stack/tracing" }
 linkerd-tls = { path = "../../tls" }
 linkerd-trace-context = { path = "../../trace-context" }
 regex = "1.0.0"
+serde_json = "1"
 thiserror = "1.0"
 tokio = { version = "1", features = ["macros", "sync", "parking_lot"]}
 tokio-stream = { version = "0.1.5", features = ["time"] }

linkerd/app/core/src/admin/level.rs

@@ -0,0 +1,52 @@
+use hyper::{
+    body::{Buf, HttpBody},
+    Body,
+};
+use linkerd_error::Error;
+use linkerd_tracing::level::Handle;
+use std::io;
+
+pub(super) async fn serve<B>(
+    level: &Handle,
+    req: http::Request<B>,
+) -> Result<http::Response<Body>, Error>
+where
+    B: HttpBody,
+    B::Error: Into<Error>,
+{
+    let mk_rsp = |status: http::StatusCode, body: Body| -> http::Response<Body> {
+        http::Response::builder()
+            .status(status)
+            .body(body)
+            .expect("builder with known status code must not fail")
+    };
+
+    let rsp = match *req.method() {
+        http::Method::GET => {
+            let level = level.current()?;
+            mk_rsp(http::StatusCode::OK, level.into())
+        }
+
+        http::Method::PUT => {
+            let body = hyper::body::aggregate(req.into_body())
+                .await
+                .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?;
+            match level.set_from(body.chunk()) {
+                Ok(()) => mk_rsp(http::StatusCode::NO_CONTENT, Body::empty()),
+                Err(error) => {
+                    tracing::warn!(%error, "Setting log level failed");
+                    mk_rsp(http::StatusCode::BAD_REQUEST, error.into())
+                }
+            }
+        }
+
+        _ => http::Response::builder()
+            .status(http::StatusCode::METHOD_NOT_ALLOWED)
+            .header("allow", "GET")
+            .header("allow", "PUT")
+            .body(Body::empty())
+            .expect("builder with known status code must not fail"),
+    };
+
+    Ok(rsp)
+}

linkerd/app/core/src/admin/mod.rs

@@ -27,7 +27,9 @@ use std::{
 };
 use tokio::sync::mpsc;
 
+mod level;
 mod readiness;
+mod tasks;
 
 pub use self::readiness::{Latch, Readiness};
 
@@ -174,19 +176,25 @@ where
             "/ready" => Box::pin(future::ok(self.ready_rsp())),
             "/metrics" => {
                 let rsp = self.metrics.serve(req).unwrap_or_else(|error| {
-                    tracing::error!(%error, "Failed to format metrics");
+                    ::tracing::error!(%error, "Failed to format metrics");
                     Self::internal_error_rsp(error)
                 });
                 Box::pin(future::ok(rsp))
             }
             "/proxy-log-level" => {
                 if Self::client_is_localhost(&req) {
-                    let handle = self.tracing.clone();
+                    let level = self.tracing.level().cloned();
                     Box::pin(async move {
-                        handle.serve_level(req).await.or_else(|error| {
-                            tracing::error!(%error, "Failed to get/set tracing level");
-                            Ok(Self::internal_error_rsp(error))
-                        })
+                        let rsp = match level {
+                            Some(level) => {
+                                level::serve(&level, req).await.unwrap_or_else(|error| {
+                                    tracing::error!(%error, "Failed to get/set tracing level");
+                                    Self::internal_error_rsp(error)
+                                })
+                            }
+                            None => Self::not_found(),
+                        };
+                        Ok(rsp)
                     })
                 } else {
                     Box::pin(future::ok(Self::forbidden_not_localhost()))
@@ -205,13 +213,14 @@ where
             }
             path if path.starts_with("/tasks") => {
                 if Self::client_is_localhost(&req) {
-                    let handle = self.tracing.clone();
-                    Box::pin(async move {
-                        handle.serve_tasks(req).await.or_else(|error| {
+                    let rsp = match self.tracing.tasks() {
+                        Some(tasks) => tasks::serve(tasks, req).unwrap_or_else(|error| {
                             tracing::error!(%error, "Failed to fetch tasks");
-                            Ok(Self::internal_error_rsp(error))
-                        })
-                    })
+                            Self::internal_error_rsp(error)
+                        }),
+                        None => Self::not_found(),
+                    };
+                    Box::pin(future::ok(rsp))
                 } else {
                     Box::pin(future::ok(Self::forbidden_not_localhost()))
                 }