Micro-optimize Span Field recording (getsentry#642)

- Use `&'static str` as the key - Reuse a thread local buffer to format `Debug` fields This duplicates the `FieldVisitor`, as other parts of the integration still expect `String` keys.
lumeohq · Feb 26, 2024 · 5f3d9ca · 5f3d9ca
1 parent 42c5f69
commit 5f3d9ca
Show file tree

Hide file tree

Showing 19 changed files with 134 additions and 52 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -65,7 +65,7 @@ jobs:
       fail-fast: false
       matrix:
         os: [ubuntu-latest, macos-latest, windows-latest]
-        rust: [1.68.0]
+        rust: [1.73.0]
 
     name: Check / Test MSRV on ${{ matrix.os }}
     runs-on: ${{ matrix.os }}

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,15 @@
 # Changelog
 
+## Unreleased
+
+**Compatiblity**:
+
+- Raised the MSRV to **1.73**.
+
+**Improvements**:
+
+- Slightly improved overhead of the `tracing` layer. ([#642](https://github.com/getsentry/sentry-rust/pull/642))
+
 ## 0.32.2
 
 ### Various fixes & improvements

diff --git a/README.md b/README.md
@@ -93,14 +93,10 @@ best API and adding new features.
 We currently only verify this crate against a recent version of Sentry hosted on [sentry.io](https://sentry.io/) but it
 should work with on-prem Sentry versions 20.6 and later.
 
-The **Minimum Supported Rust Version** is currently at _1.68.0_.
-The Sentry crates will support a _6 months_ old Rust version at time of release,
+The **Minimum Supported Rust Version** is currently at _1.73.0_.
+The Sentry crates tries to support a _6 months_ old Rust version at time of release,
 and the MSRV will be increased in accordance with its dependencies.
 
-**Note**: Due to the misconfiguration in our workflow that verified the MSRV builds,
-we had to bump it all the way to _1.67.0_, breaking our own _6 months_ old rule.
-As of version _0.30.0_, the workflow has been fixed, and the MSRV will be enforced correctly.
-
 ## Resources
 
 - [Discord](https://discord.gg/ez5KZN7) server for project discussions.

diff --git a/clippy.toml b/clippy.toml
@@ -1 +1 @@
-msrv = "1.68.0"
+msrv = "1.73.0"
diff --git a/sentry-actix/Cargo.toml b/sentry-actix/Cargo.toml
@@ -10,12 +10,14 @@ description = """
 Sentry client extension for actix-web 3.
 """
 edition = "2021"
-rust-version = "1.68"
+rust-version = "1.73"
 
 [dependencies]
 actix-web = { version = "4", default-features = false }
 futures-util = { version = "0.3.5", default-features = false }
-sentry-core = { version = "0.32.2", path = "../sentry-core", default-features = false, features = ["client"] }
+sentry-core = { version = "0.32.2", path = "../sentry-core", default-features = false, features = [
+    "client",
+] }
 
 [dev-dependencies]
 actix-web = { version = "4" }

diff --git a/sentry-anyhow/Cargo.toml b/sentry-anyhow/Cargo.toml
@@ -10,7 +10,7 @@ description = """
 Sentry integration for anyhow.
 """
 edition = "2021"
-rust-version = "1.68"
+rust-version = "1.73"
 
 [features]
 default = ["backtrace"]

diff --git a/sentry-backtrace/Cargo.toml b/sentry-backtrace/Cargo.toml
@@ -10,10 +10,13 @@ description = """
 Sentry integration and utilities for dealing with stacktraces.
 """
 edition = "2021"
-rust-version = "1.68"
+rust-version = "1.73"
 
 [dependencies]
 backtrace = "0.3.44"
 once_cell = "1"
-regex = { version = "1.5.5", default-features = false, features = ["std", "unicode-perl"] }
+regex = { version = "1.5.5", default-features = false, features = [
+    "std",
+    "unicode-perl",
+] }
 sentry-core = { version = "0.32.2", path = "../sentry-core" }
diff --git a/sentry-contexts/Cargo.toml b/sentry-contexts/Cargo.toml
@@ -11,7 +11,7 @@ Sentry integration for os, device, and rust contexts.
 """
 build = "build.rs"
 edition = "2021"
-rust-version = "1.68"
+rust-version = "1.73"
 
 [dependencies]
 sentry-core = { version = "0.32.2", path = "../sentry-core" }

diff --git a/sentry-core/Cargo.toml b/sentry-core/Cargo.toml
@@ -10,7 +10,7 @@ description = """
 Core sentry library used for instrumentation and integration development.
 """
 edition = "2021"
-rust-version = "1.68"
+rust-version = "1.73"
 
 [package.metadata.docs.rs]
 all-features = true
@@ -43,7 +43,10 @@ uuid = { version = "1.0.0", features = ["v4", "serde"], optional = true }
 # Because we re-export all the public API in `sentry`, we actually run all the
 # doctests using the `sentry` crate. This also takes care of the doctest
 # limitation documented in https://github.com/rust-lang/rust/issues/45599.
-sentry = { path = "../sentry", default-features = false, features = ["test", "transport"] }
+sentry = { path = "../sentry", default-features = false, features = [
+    "test",
+    "transport",
+] }
 anyhow = "1.0.30"
 criterion = "0.5"
 futures = "0.3.24"

diff --git a/sentry-debug-images/Cargo.toml b/sentry-debug-images/Cargo.toml
@@ -10,7 +10,7 @@ description = """
 Sentry integration that adds the list of loaded libraries to events.
 """
 edition = "2021"
-rust-version = "1.68"
+rust-version = "1.73"
 
 [dependencies]
 findshlibs = "=0.10.2"

diff --git a/sentry-log/Cargo.toml b/sentry-log/Cargo.toml
@@ -10,7 +10,7 @@ description = """
 Sentry integration for log and env_logger crates.
 """
 edition = "2021"
-rust-version = "1.68"
+rust-version = "1.73"
 
 [dependencies]
 sentry-core = { version = "0.32.2", path = "../sentry-core" }

diff --git a/sentry-panic/Cargo.toml b/sentry-panic/Cargo.toml
@@ -10,7 +10,7 @@ description = """
 Sentry integration for capturing panics.
 """
 edition = "2021"
-rust-version = "1.68"
+rust-version = "1.73"
 
 [dependencies]
 sentry-core = { version = "0.32.2", path = "../sentry-core" }

diff --git a/sentry-slog/Cargo.toml b/sentry-slog/Cargo.toml
@@ -10,7 +10,7 @@ description = """
 Sentry integration for the slog crate.
 """
 edition = "2021"
-rust-version = "1.68"
+rust-version = "1.73"
 
 [dependencies]
 sentry-core = { version = "0.32.2", path = "../sentry-core" }

diff --git a/sentry-tower/Cargo.toml b/sentry-tower/Cargo.toml
@@ -10,7 +10,7 @@ description = """
 Sentry integration for tower-based crates.
 """
 edition = "2021"
-rust-version = "1.68"
+rust-version = "1.73"
 
 [package.metadata.docs.rs]
 all-features = true
@@ -25,7 +25,9 @@ tower-layer = "0.3"
 tower-service = "0.3"
 http = { version = "1.0.0", optional = true }
 pin-project = { version = "1.0.10", optional = true }
-sentry-core = { version = "0.32.2", path = "../sentry-core", default-features = false, features = ["client"] }
+sentry-core = { version = "0.32.2", path = "../sentry-core", default-features = false, features = [
+    "client",
+] }
 url = { version = "2.2.2", optional = true }
 
 [dev-dependencies]

diff --git a/sentry-tracing/Cargo.toml b/sentry-tracing/Cargo.toml
@@ -10,7 +10,7 @@ description = """
 Sentry integration for tracing and tracing-subscriber crates.
 """
 edition = "2021"
-rust-version = "1.68"
+rust-version = "1.73"
 
 [package.metadata.docs.rs]
 all-features = true
@@ -20,9 +20,13 @@ default = []
 backtrace = ["dep:sentry-backtrace"]
 
 [dependencies]
-sentry-core = { version = "0.32.2", path = "../sentry-core", features = ["client"] }
+sentry-core = { version = "0.32.2", path = "../sentry-core", features = [
+    "client",
+] }
 tracing-core = "0.1"
-tracing-subscriber = { version = "0.3.1", default-features = false, features = ["std"] }
+tracing-subscriber = { version = "0.3.1", default-features = false, features = [
+    "std",
+] }
 sentry-backtrace = { version = "0.32.2", path = "../sentry-backtrace", optional = true }
 
 [dev-dependencies]

diff --git a/sentry-tracing/src/converters.rs b/sentry-tracing/src/converters.rs
@@ -4,7 +4,7 @@ use std::error::Error;
 use sentry_core::protocol::{Event, Exception, Mechanism, Thread, Value};
 use sentry_core::{event_from_error, Breadcrumb, Level, TransactionOrSpan};
 use tracing_core::field::{Field, Visit};
-use tracing_core::{span, Subscriber};
+use tracing_core::Subscriber;
 use tracing_subscriber::layer::Context;
 use tracing_subscriber::registry::LookupSpan;
 
@@ -42,7 +42,10 @@ fn extract_event_data(event: &tracing_core::Event) -> (Option<String>, FieldVisi
         // When #[instrument(err)] is used the event does not have a message attached to it.
         // the error message is attached to the field "error".
         .or_else(|| visitor.json_values.remove("error"))
-        .and_then(|v| v.as_str().map(|s| s.to_owned()));
+        .and_then(|v| match v {
+            Value::String(s) => Some(s),
+            _ => None,
+        });
 
     (message, visitor)
 }
@@ -93,22 +96,6 @@ where
     (message, visitor)
 }
 
-/// Extracts the message and metadata from a span
-pub(crate) fn extract_span_data(
-    attrs: &span::Attributes,
-) -> (Option<String>, BTreeMap<String, Value>) {
-    let mut data = FieldVisitor::default();
-    attrs.record(&mut data);
-
-    // Find message of the span, if any
-    let message = data
-        .json_values
-        .remove("message")
-        .and_then(|v| v.as_str().map(|s| s.to_owned()));
-
-    (message, data.json_values)
-}
-
 /// Records all fields of [`tracing_core::Event`] for easy access
 #[derive(Default)]
 pub(crate) struct FieldVisitor {

diff --git a/sentry-tracing/src/layer.rs b/sentry-tracing/src/layer.rs
@@ -1,5 +1,10 @@
+use std::cell::RefCell;
+use std::collections::BTreeMap;
+
+use sentry_core::protocol::Value;
 use sentry_core::{Breadcrumb, TransactionOrSpan};
-use tracing_core::{span, Event, Level, Metadata, Subscriber};
+use tracing_core::field::Visit;
+use tracing_core::{span, Event, Field, Level, Metadata, Subscriber};
 use tracing_subscriber::layer::{Context, Layer};
 use tracing_subscriber::registry::LookupSpan;
 
@@ -213,7 +218,7 @@ where
         // Add the data from the original span to the sentry span.
         // This comes from typically the `fields` in `tracing::instrument`.
         for (key, value) in data {
-            sentry_span.set_data(&key, value);
+            sentry_span.set_data(key, value);
         }
 
         sentry_core::configure_scope(|scope| scope.set_span(Some(sentry_span.clone())));
@@ -275,3 +280,66 @@ where
 {
     Default::default()
 }
+
+/// Extracts the message and attributes from a span
+fn extract_span_data(attrs: &span::Attributes) -> (Option<String>, BTreeMap<&'static str, Value>) {
+    let mut json_values = VISITOR_BUFFER.with_borrow_mut(|debug_buffer| {
+        let mut visitor = SpanFieldVisitor {
+            debug_buffer,
+            json_values: Default::default(),
+        };
+        attrs.record(&mut visitor);
+        visitor.json_values
+    });
+
+    // Find message of the span, if any
+    let message = json_values.remove("message").and_then(|v| match v {
+        Value::String(s) => Some(s),
+        _ => None,
+    });
+
+    (message, json_values)
+}
+
+thread_local! {
+    static VISITOR_BUFFER: RefCell<String> = const { RefCell::new(String::new()) };
+}
+
+/// Records all span fields into a `BTreeMap`, reusing a mutable `String` as buffer.
+struct SpanFieldVisitor<'s> {
+    debug_buffer: &'s mut String,
+    json_values: BTreeMap<&'static str, Value>,
+}
+
+impl SpanFieldVisitor<'_> {
+    fn record<T: Into<Value>>(&mut self, field: &Field, value: T) {
+        self.json_values.insert(field.name(), value.into());
+    }
+}
+
+impl Visit for SpanFieldVisitor<'_> {
+    fn record_i64(&mut self, field: &Field, value: i64) {
+        self.record(field, value);
+    }
+
+    fn record_u64(&mut self, field: &Field, value: u64) {
+        self.record(field, value);
+    }
+
+    fn record_bool(&mut self, field: &Field, value: bool) {
+        self.record(field, value);
+    }
+
+    fn record_str(&mut self, field: &Field, value: &str) {
+        self.record(field, value);
+    }
+
+    fn record_debug(&mut self, field: &Field, value: &dyn std::fmt::Debug) {
+        use std::fmt::Write;
+        self.debug_buffer.reserve(128);
+        write!(self.debug_buffer, "{value:?}").unwrap();
+        self.json_values
+            .insert(field.name(), self.debug_buffer.as_str().into());
+        self.debug_buffer.clear();
+    }
+}
diff --git a/sentry-types/Cargo.toml b/sentry-types/Cargo.toml
@@ -11,7 +11,7 @@ Common reusable types for implementing the sentry.io protocol.
 """
 keywords = ["sentry", "protocol"]
 edition = "2021"
-rust-version = "1.68"
+rust-version = "1.73"
 
 [package.metadata.docs.rs]
 all-features = true

diff --git a/sentry/Cargo.toml b/sentry/Cargo.toml
@@ -10,7 +10,7 @@ description = """
 Sentry (getsentry.com) client for rust ;)
 """
 edition = "2021"
-rust-version = "1.68"
+rust-version = "1.73"
 autoexamples = true
 
 # To build locally:
@@ -49,10 +49,12 @@ surf = ["surf/curl-client", "http-client", "httpdate", "isahc", "tokio"]
 ureq = ["dep:ureq", "httpdate"]
 # transport settings
 native-tls = ["dep:native-tls", "reqwest?/default-tls", "ureq?/native-tls"]
-rustls =     ["dep:rustls",     "reqwest?/rustls-tls",  "ureq?/tls",        "webpki-roots"]
+rustls = ["dep:rustls", "reqwest?/rustls-tls", "ureq?/tls", "webpki-roots"]
 
 [dependencies]
-sentry-core = { version = "0.32.2", path = "../sentry-core", features = ["client"] }
+sentry-core = { version = "0.32.2", path = "../sentry-core", features = [
+    "client",
+] }
 sentry-anyhow = { version = "0.32.2", path = "../sentry-anyhow", optional = true }
 sentry-backtrace = { version = "0.32.2", path = "../sentry-backtrace", optional = true }
 sentry-contexts = { version = "0.32.2", path = "../sentry-contexts", optional = true }
@@ -63,7 +65,10 @@ sentry-slog = { version = "0.32.2", path = "../sentry-slog", optional = true }
 sentry-tower = { version = "0.32.2", path = "../sentry-tower", optional = true }
 sentry-tracing = { version = "0.32.2", path = "../sentry-tracing", optional = true }
 log = { version = "0.4.8", optional = true, features = ["std"] }
-reqwest = { version = "0.11", optional = true, features = ["blocking", "json"], default-features = false }
+reqwest = { version = "0.11", optional = true, features = [
+    "blocking",
+    "json",
+], default-features = false }
 curl = { version = "0.4.25", optional = true }
 httpdate = { version = "1.0.0", optional = true }
 surf = { version = "2.0.0", optional = true, default-features = false }
@@ -73,7 +78,9 @@ serde_json = { version = "1.0.48", optional = true }
 tokio = { version = "1.0", features = ["rt"], optional = true }
 ureq = { version = "2.7.0", optional = true, default-features = false }
 native-tls = { version = "0.2.8", optional = true }
-rustls = { version = "0.21.2", optional = true, features = ["dangerous_configuration"] }
+rustls = { version = "0.21.2", optional = true, features = [
+    "dangerous_configuration",
+] }
 webpki-roots = { version = "0.25.1", optional = true }
 
 [dev-dependencies]
@@ -86,7 +93,7 @@ actix-web = { version = "4", default-features = false }
 anyhow = { version = "1.0.30" }
 log = { version = "0.4.8", features = ["std"] }
 pretty_env_logger = "0.5.0"
-slog = {version = "2.5.2" }
+slog = { version = "2.5.2" }
 tokio = { version = "1.0", features = ["macros"] }
 tower = { version = "0.4", features = ["util"] }
 tracing = { version = "0.1" }