diff --git a/Cargo.lock b/Cargo.lock index c8042ba43ec8d..a4e9c22f704cd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5870,27 +5870,6 @@ dependencies = [ "winapi 0.3.8", ] -[[package]] -name = "rental" -version = "0.5.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8545debe98b2b139fb04cad8618b530e9b07c152d99a5de83c860b877d67847f" -dependencies = [ - "rental-impl", - "stable_deref_trait", -] - -[[package]] -name = "rental-impl" -version = "0.5.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "475e68978dc5b743f2f40d8e0a8fdc83f1c5e78cbf4b8fa5e74e73beebc340de" -dependencies = [ - "proc-macro2", - "quote 1.0.6", - "syn 1.0.33", -] - [[package]] name = "ring" version = "0.16.12" @@ -8158,7 +8137,6 @@ name = "sp-tracing" version = "2.0.0-rc4" dependencies = [ "log", - "rental", "tracing", ] diff --git a/docs/CODEOWNERS b/docs/CODEOWNERS index d9342de399503..cb1e012ffd6fe 100644 --- a/docs/CODEOWNERS +++ b/docs/CODEOWNERS @@ -66,3 +66,8 @@ # Prometheus endpoint /utils/prometheus/ @mxinden + +# Tracing +/client/tracing/ @mattrutherford +/primitives/tracing/src/proxy.rs @mattrutherford +/primitives/tracing/src/proxied_span.rs @mattrutherford diff --git a/primitives/tracing/Cargo.toml b/primitives/tracing/Cargo.toml index 30808a6c0e4ea..b2cd06eef4604 100644 --- a/primitives/tracing/Cargo.toml +++ b/primitives/tracing/Cargo.toml @@ -13,9 +13,8 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] tracing = { version = "0.1.13", optional = true } -rental = { version = "0.5.5", optional = true } log = { version = "0.4.8", optional = true } [features] default = [ "std" ] -std = [ "tracing", "rental", "log" ] +std = [ "tracing", "log" ] diff --git a/primitives/tracing/src/lib.rs b/primitives/tracing/src/lib.rs index e82d8861cd3f5..6d97852e13f36 100644 --- a/primitives/tracing/src/lib.rs +++ b/primitives/tracing/src/lib.rs @@ -30,10 +30,6 @@ //! the associated Fields mentioned above. #![cfg_attr(not(feature = "std"), no_std)] -#[cfg(feature = "std")] -#[macro_use] -extern crate rental; - #[cfg(feature = "std")] #[doc(hidden)] pub use tracing; @@ -41,6 +37,9 @@ pub use tracing; #[cfg(feature = "std")] pub mod proxy; +#[cfg(feature = "std")] +mod proxied_span; + #[cfg(feature = "std")] use std::sync::atomic::{AtomicBool, Ordering}; diff --git a/primitives/tracing/src/proxied_span.rs b/primitives/tracing/src/proxied_span.rs new file mode 100644 index 0000000000000..0cc61eda90be5 --- /dev/null +++ b/primitives/tracing/src/proxied_span.rs @@ -0,0 +1,68 @@ +// Copyright 2020 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +//! Provides the struct `ProxiedSpan`, which encapsulates the necessary +//! associated unsafe code and provides a safe interface to the proxy. + +use std::mem::{transmute, ManuallyDrop}; +use std::pin::Pin; +use tracing::{span::Entered, Span}; + +/// Container for a proxied tracing Span and it's associated guard +pub struct ProxiedSpan { + id: u64, + // Crucial that `guard` must be dropped first - ensured by `drop` impl. + guard: ManuallyDrop>, + span: ManuallyDrop>>, +} + +impl ProxiedSpan { + /// Enter the supplied span and return a new instance of ProxiedTrace containing it + pub fn enter_span(id: u64, span: Pin>) -> Self { + let span = ManuallyDrop::new(span); + // Transmute to static lifetime to enable guard to be stored + // along with the span that it references + let guard = unsafe { + ManuallyDrop::new(transmute::, Entered<'static>>(span.enter())) + }; + + ProxiedSpan { + id, + guard, + span, + } + } + + /// Return copy of the span id + pub fn id(&self) -> u64 { + self.id + } + + /// Return immutable reference to the span + pub fn span(&self) -> &Span { + &*self.span + } +} + +impl Drop for ProxiedSpan { + fn drop(&mut self) { + unsafe { + // Ensure guard is dropped before span to prevent invalid reference + ManuallyDrop::drop(&mut self.guard); + ManuallyDrop::drop(&mut self.span); + } + } +} diff --git a/primitives/tracing/src/proxy.rs b/primitives/tracing/src/proxy.rs index 270f57aaa69f0..fa3c74871120b 100644 --- a/primitives/tracing/src/proxy.rs +++ b/primitives/tracing/src/proxy.rs @@ -17,8 +17,8 @@ //! Proxy to allow entering tracing spans from wasm. //! //! Use `enter_span` and `exit_span` to surround the code that you wish to trace -use rental; use tracing::info_span; +use crate::proxied_span::ProxiedSpan; /// Used to identify a proxied WASM trace pub const WASM_TRACE_IDENTIFIER: &'static str = "WASM_TRACE"; @@ -29,21 +29,11 @@ pub const WASM_NAME_KEY: &'static str = "proxied_wasm_name"; const MAX_SPANS_LEN: usize = 1000; -rental! { - pub mod rent_span { - #[rental] - pub struct SpanAndGuard { - span: Box, - guard: tracing::span::Entered<'span>, - } - } -} - /// Requires a tracing::Subscriber to process span traces, /// this is available when running with client (and relevant cli params). pub struct TracingProxy { next_id: u64, - spans: Vec<(u64, rent_span::SpanAndGuard)>, + spans: Vec, } impl Drop for TracingProxy { @@ -53,8 +43,8 @@ impl Drop for TracingProxy { target: "tracing", "Dropping TracingProxy with {} un-exited spans, marking as not valid", self.spans.len() ); - while let Some((_, mut sg)) = self.spans.pop() { - sg.rent_all_mut(|s| { s.span.record("is_valid_trace", &false); }); + while let Some(proxied_span) = self.spans.pop() { + proxied_span.span().record("is_valid_trace", &false); } } } @@ -75,13 +65,17 @@ impl TracingProxy { pub fn enter_span(&mut self, proxied_wasm_target: &str, proxied_wasm_name: &str) -> u64 { // The identifiers `proxied_wasm_target` and `proxied_wasm_name` must match their associated const, // WASM_TARGET_KEY and WASM_NAME_KEY. - let span = info_span!(WASM_TRACE_IDENTIFIER, is_valid_trace = true, proxied_wasm_target, proxied_wasm_name); - self.next_id += 1; - let sg = rent_span::SpanAndGuard::new( - Box::new(span), - |span| span.enter(), + let span = Box::pin( + info_span!( + WASM_TRACE_IDENTIFIER, + is_valid_trace = true, + proxied_wasm_target, + proxied_wasm_name + ) ); - self.spans.push((self.next_id, sg)); + self.next_id += 1; + let proxied_span = ProxiedSpan::enter_span(self.next_id, span); + self.spans.push(proxied_span); if self.spans.len() > MAX_SPANS_LEN { // This is to prevent unbounded growth of Vec and could mean one of the following: // 1. Too many nested spans, or MAX_SPANS_LEN is too low. @@ -90,29 +84,29 @@ impl TracingProxy { target: "tracing", "TracingProxy MAX_SPANS_LEN exceeded, removing oldest span." ); - let mut sg = self.spans.remove(0).1; - sg.rent_all_mut(|s| { s.span.record("is_valid_trace", &false); }); + let proxied_span = self.spans.remove(0); + proxied_span.span().record("is_valid_trace", &false); } self.next_id } /// Exit a span by dropping it along with it's associated guard. pub fn exit_span(&mut self, id: u64) { - if self.spans.last().map(|l| id > l.0).unwrap_or(true) { + if self.spans.last().map(|proxied_span| id > proxied_span.id()).unwrap_or(true) { log::warn!(target: "tracing", "Span id not found in TracingProxy: {}", id); return; } - let mut last_span = self.spans.pop().expect("Just checked that there is an element to pop; qed"); - while id < last_span.0 { + let mut proxied_span = self.spans.pop().expect("Just checked that there is an element to pop; qed"); + while id < proxied_span.id() { log::warn!( target: "tracing", "TracingProxy Span ids not equal! id parameter given: {}, last span: {}", id, - last_span.0, + proxied_span.id(), ); - last_span.1.rent_all_mut(|s| { s.span.record("is_valid_trace", &false); }); - if let Some(s) = self.spans.pop() { - last_span = s; + proxied_span.span().record("is_valid_trace", &false); + if let Some(ps) = self.spans.pop() { + proxied_span = ps; } else { log::warn!(target: "tracing", "Span id not found in TracingProxy {}", id); return; @@ -140,7 +134,7 @@ mod tests { let _spans = create_spans(&mut proxy, MAX_SPANS_LEN + 10); assert_eq!(proxy.spans.len(), MAX_SPANS_LEN); // ensure oldest spans removed - assert_eq!(proxy.spans[0].0, 11); + assert_eq!(proxy.spans[0].id(), 11); } #[test]