Skip to content

Commit 0773368

Browse files
authored
feat(service): Add span instrumentation to backends (#542)
Backend operations now appear as real child spans in Sentry, turning the existing per-request and per-task transactions into full trace waterfalls. They were empty before because the sentry-tracing layer only converts `INFO`+ spans by default; the logging layer now forwards `DEBUG`+ spans (`TRACE` still ignored) and the backend spans moved to `debug`. GCS/S3 HTTP calls and BigTable RPCs each get a per-attempt span covering the real request duration (method/URL/status, or action/gRPC status), and the tiered, changelog, and per-backend operations are annotated so the waterfall reads top-down. Spans carry small primitive fields (object id, range, part numbers, URLs); payloads, streams, and metadata are excluded. Observability-only — no behavior changes and console logging is untouched.
1 parent 536e283 commit 0773368

9 files changed

Lines changed: 138 additions & 59 deletions

File tree

objectstore-log/src/subscriber.rs

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,24 @@ include!(concat!(env!("OUT_DIR"), "/constants.gen.rs"));
1515
///
1616
/// When built with the `sentry` feature, also attaches a Sentry tracing layer if the Sentry client
1717
/// has been initialized. `ERROR` and `WARN` events become Sentry events; `INFO`/`DEBUG` become
18-
/// Sentry logs; `TRACE` is ignored.
18+
/// Sentry logs; `TRACE` events are ignored. `ERROR`..`DEBUG` spans become Sentry spans; `TRACE`
19+
/// spans are ignored.
1920
pub fn init(config: &LoggingConfig) {
2021
#[cfg(feature = "sentry")]
2122
let sentry_layer = sentry::Hub::current()
2223
.client()
2324
.filter(|c| c.is_enabled())
2425
.map(|_| {
2526
use sentry::integrations::tracing as sentry_tracing;
26-
sentry_tracing::layer().event_filter(|metadata| match *metadata.level() {
27-
Level::ERROR | Level::WARN => {
28-
sentry_tracing::EventFilter::Event | sentry_tracing::EventFilter::Log
29-
}
30-
Level::INFO | Level::DEBUG => sentry_tracing::EventFilter::Log,
31-
Level::TRACE => sentry_tracing::EventFilter::Ignore,
32-
})
27+
sentry_tracing::layer()
28+
.event_filter(|metadata| match *metadata.level() {
29+
Level::ERROR | Level::WARN => {
30+
sentry_tracing::EventFilter::Event | sentry_tracing::EventFilter::Log
31+
}
32+
Level::INFO | Level::DEBUG => sentry_tracing::EventFilter::Log,
33+
Level::TRACE => sentry_tracing::EventFilter::Ignore,
34+
})
35+
.span_filter(|metadata| !matches!(*metadata.level(), Level::TRACE))
3336
});
3437

3538
let format = tracing_subscriber::fmt::layer()

objectstore-service/src/backend/bigtable.rs

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ use objectstore_types::metadata::{ExpirationPolicy, Metadata};
4141
use objectstore_types::range::{ByteRange, ContentRange};
4242
use serde::{Deserialize, Serialize};
4343
use tonic::Code;
44+
use tracing::Instrument;
4445

4546
use crate::backend::common::{
4647
Backend, DeleteResponse, GetResponse, HighVolumeBackend, MetadataResponse, PutResponse,
@@ -740,6 +741,7 @@ impl BigTableBackend {
740741
/// Reads a single row by key, returning parsed row data.
741742
///
742743
/// Returns `None` if the row is absent or has expired.
744+
#[tracing::instrument(level = "debug", fields(action), skip_all)]
743745
async fn read_row(
744746
&self,
745747
path: &[u8],
@@ -776,6 +778,7 @@ impl BigTableBackend {
776778
})
777779
}
778780

781+
#[tracing::instrument(level = "debug", fields(action), skip_all)]
779782
async fn mutate(
780783
&self,
781784
path: Vec<u8>,
@@ -821,6 +824,7 @@ impl BigTableBackend {
821824
/// Best-effort TTI bump for a row.
822825
///
823826
/// If the payload isn't loaded, it will be fetched. Failures are ignored silently.
827+
#[tracing::instrument(level = "debug", fields(?hv_id, loaded), skip_all)]
824828
async fn bump_tti(&self, path: Vec<u8>, row: &RowData, loaded: bool, hv_id: &ObjectId) {
825829
let expiration_policy = row.expiration_policy();
826830

@@ -860,6 +864,7 @@ impl BigTableBackend {
860864
}
861865

862866
/// Executes a `CheckAndMutateRow` request.
867+
#[tracing::instrument(level = "debug", fields(action = context), skip_all)]
863868
async fn check_and_mutate(
864869
&self,
865870
row_key: Vec<u8>,
@@ -898,7 +903,7 @@ impl Backend for BigTableBackend {
898903
"bigtable"
899904
}
900905

901-
#[tracing::instrument(level = "trace", fields(?id), skip_all)]
906+
#[tracing::instrument(level = "debug", fields(?id), skip_all)]
902907
async fn put_object(
903908
&self,
904909
id: &ObjectId,
@@ -919,7 +924,7 @@ impl Backend for BigTableBackend {
919924
Ok(())
920925
}
921926

922-
#[tracing::instrument(level = "trace", fields(?id), skip_all)]
927+
#[tracing::instrument(level = "debug", skip(self))]
923928
async fn get_object(&self, id: &ObjectId, range: Option<ByteRange>) -> Result<GetResponse> {
924929
match self.get_tiered_object(id, range).await? {
925930
TieredGet::Object(metadata, content_range, payload) => {
@@ -930,7 +935,7 @@ impl Backend for BigTableBackend {
930935
}
931936
}
932937

933-
#[tracing::instrument(level = "trace", fields(?id), skip_all)]
938+
#[tracing::instrument(level = "debug", skip(self))]
934939
async fn get_metadata(&self, id: &ObjectId) -> Result<MetadataResponse> {
935940
match self.get_tiered_metadata(id).await? {
936941
TieredMetadata::Object(metadata) => Ok(Some(metadata)),
@@ -939,7 +944,7 @@ impl Backend for BigTableBackend {
939944
}
940945
}
941946

942-
#[tracing::instrument(level = "trace", fields(?id), skip_all)]
947+
#[tracing::instrument(level = "debug", skip(self))]
943948
async fn delete_object(&self, id: &ObjectId) -> Result<DeleteResponse> {
944949
objectstore_log::debug!("Deleting from Bigtable backend");
945950

@@ -952,7 +957,7 @@ impl Backend for BigTableBackend {
952957

953958
#[async_trait::async_trait]
954959
impl HighVolumeBackend for BigTableBackend {
955-
#[tracing::instrument(level = "trace", fields(?id), skip_all)]
960+
#[tracing::instrument(level = "debug", fields(?id), skip_all)]
956961
async fn put_non_tombstone(
957962
&self,
958963
id: &ObjectId,
@@ -1000,7 +1005,7 @@ impl HighVolumeBackend for BigTableBackend {
10001005
Err(Error::generic("BigTable: race loop in put_non_tombstone"))
10011006
}
10021007

1003-
#[tracing::instrument(level = "trace", fields(?id), skip_all)]
1008+
#[tracing::instrument(level = "debug", skip(self))]
10041009
async fn get_tiered_object(
10051010
&self,
10061011
id: &ObjectId,
@@ -1037,7 +1042,7 @@ impl HighVolumeBackend for BigTableBackend {
10371042
})
10381043
}
10391044

1040-
#[tracing::instrument(level = "trace", fields(?id), skip_all)]
1045+
#[tracing::instrument(level = "debug", skip(self))]
10411046
async fn get_tiered_metadata(&self, id: &ObjectId) -> Result<TieredMetadata> {
10421047
objectstore_log::debug!("Reading metadata from Bigtable backend");
10431048
let path = id.as_storage_path().to_string().into_bytes();
@@ -1066,7 +1071,7 @@ impl HighVolumeBackend for BigTableBackend {
10661071
})
10671072
}
10681073

1069-
#[tracing::instrument(level = "trace", fields(?id), skip_all)]
1074+
#[tracing::instrument(level = "debug", skip(self))]
10701075
async fn delete_non_tombstone(&self, id: &ObjectId) -> Result<Option<Tombstone>> {
10711076
objectstore_log::debug!("Conditional delete from Bigtable backend");
10721077

@@ -1110,7 +1115,7 @@ impl HighVolumeBackend for BigTableBackend {
11101115
))
11111116
}
11121117

1113-
#[tracing::instrument(level = "trace", fields(?id), skip_all)]
1118+
#[tracing::instrument(level = "debug", skip(self, write))]
11141119
async fn compare_and_write(
11151120
&self,
11161121
id: &ObjectId,
@@ -1205,7 +1210,26 @@ where
12051210
let mut retry_count = 0usize;
12061211

12071212
loop {
1208-
match f().await {
1213+
let attempt_span = tracing::debug_span!(
1214+
"bigtable.request",
1215+
action = context,
1216+
grpc.status = tracing::field::Empty,
1217+
);
1218+
let attempt = async {
1219+
let result = f().await;
1220+
let span = tracing::Span::current();
1221+
match &result {
1222+
Ok(_) => span.record("grpc.status", "ok"),
1223+
Err(BigTableError::RpcError(status)) => {
1224+
span.record("grpc.status", tracing::field::debug(status.code()))
1225+
}
1226+
// Non-RPC error; the error event carries the details.
1227+
Err(_) => &span,
1228+
};
1229+
result
1230+
};
1231+
1232+
match attempt.instrument(attempt_span).await {
12091233
Ok(res) => return Ok(res),
12101234
Err(e) if retry_count >= REQUEST_RETRY_COUNT || !is_retryable(&e) => {
12111235
objectstore_metrics::count!("bigtable.failures", action = context);

objectstore-service/src/backend/changelog.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ impl ChangeManager {
121121
///
122122
/// When the [`ChangeGuard`] is dropped, a background process is spawned to
123123
/// clean up any unreferenced objects in LT storage.
124+
#[tracing::instrument(level = "debug", fields(id = ?change.id, new = ?change.new, old = ?change.old), skip_all)]
124125
pub async fn record(self: Arc<Self>, change: Change) -> Result<ChangeGuard> {
125126
let token = self.tracker.token();
126127

@@ -143,6 +144,7 @@ impl ChangeManager {
143144
/// Behaves like [`Self::record`], except that the guard is created in the `Assembling` state.
144145
/// Unlike other states, this guard does nothing on drop, leaving the burden of cleaning up to
145146
/// the [`ChangeLog`].
147+
#[tracing::instrument(level = "debug", fields(id = ?change.id, new = ?change.new, old = ?change.old), skip_all)]
146148
pub async fn record_assembling(self: Arc<Self>, change: Change) -> Result<ChangeGuard> {
147149
let token = self.tracker.token();
148150

objectstore-service/src/backend/response.rs renamed to objectstore-service/src/backend/extensions.rs

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,46 @@
1-
//! HTTP response status checking with error body parsing.
1+
//! Extension traits for `reqwest` requests and responses.
22
//!
3-
//! Provides [`ResponseExt`], an extension trait that replaces
4-
//! [`reqwest::Response::error_for_status`] with a version that reads the
5-
//! response body on 4xx/5xx errors and parses the structured error code and
6-
//! message from it (JSON for GCS JSON API, XML for GCS XML API and S3).
3+
//! Provides [`SendTraced`], which sends a request inside a tracing span, and
4+
//! [`ResponseExt`], which replaces [`reqwest::Response::error_for_status`] with a
5+
//! version that reads the response body on 4xx/5xx errors and parses the
6+
//! structured error code and message from it (JSON for GCS JSON API, XML for GCS
7+
//! XML API and S3).
78
89
use reqwest::{Response, header};
910
use serde::Deserialize;
11+
use tracing::Instrument;
1012

1113
use crate::error::{BackendDetail, Error, Result};
1214
use crate::stream;
1315

16+
/// Extension trait that sends a request inside a tracing span.
17+
pub trait SendTraced {
18+
/// Sends the request, wrapping it in a span that covers the full request
19+
/// duration and records the response status code.
20+
async fn send_traced(self) -> reqwest::Result<reqwest::Response>;
21+
}
22+
23+
impl SendTraced for reqwest::RequestBuilder {
24+
async fn send_traced(self) -> reqwest::Result<reqwest::Response> {
25+
let (client, request) = self.build_split();
26+
let request = request?;
27+
let span = tracing::debug_span!(
28+
"http.request",
29+
method = %request.method(),
30+
url = %request.url(),
31+
http.status_code = tracing::field::Empty,
32+
);
33+
let send_future = async {
34+
let response = client.execute(request).await;
35+
if let Ok(response) = &response {
36+
tracing::Span::current().record("http.status_code", response.status().as_u16());
37+
}
38+
response
39+
};
40+
send_future.instrument(span).await
41+
}
42+
}
43+
1444
/// GCS JSON API error envelope (`{"error": {"message": "...", ...}}`).
1545
#[derive(Deserialize)]
1646
struct JsonApiError {

0 commit comments

Comments
 (0)