Skip to content

Commit eedbb03

Browse files
haphungwAI Bot
authored andcommitted
feat(o11y): use local crate name as tracing target
1 parent 2cc182b commit eedbb03

File tree

3 files changed

+87
-122
lines changed

3 files changed

+87
-122
lines changed

src/gax-internal/src/observability.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,5 @@ mod client_signals;
4343

4444
#[cfg(google_cloud_unstable_tracing)]
4545
pub use client_signals::{
46-
ClientRequestAttributes, DurationMetric, RequestRecorder, WithClientLogging, WithClientMetric,
47-
WithClientSpan,
46+
ClientRequestAttributes, DurationMetric, RequestRecorder, WithClientMetric, WithClientSpan,
4847
};

src/gax-internal/src/observability/client_signals.rs

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ mod with_client_span;
2020

2121
pub use duration_metric::DurationMetric;
2222
pub use recorder::{ClientRequestAttributes, RequestRecorder};
23-
pub use with_client_logging::WithClientLogging;
23+
2424
pub use with_client_metric::WithClientMetric;
2525
pub use with_client_span::WithClientSpan;
2626

@@ -101,7 +101,7 @@ macro_rules! client_request_signals {
101101
span.clone(),
102102
$crate::observability::WithClientMetric::new(
103103
$metric,
104-
$crate::observability::WithClientLogging::new($inner),
104+
$crate::with_client_logging!($inner),
105105
),
106106
))
107107
.instrument(span.clone());
@@ -112,7 +112,7 @@ macro_rules! client_request_signals {
112112
#[cfg(test)]
113113
mod tests {
114114
use super::duration_metric::BOUNDARIES;
115-
use super::with_client_logging::{NAME, TARGET};
115+
116116
use super::{ClientRequestAttributes, RequestRecorder};
117117
use crate::observability::DurationMetric;
118118
use crate::options::InstrumentationClientInfo;
@@ -288,11 +288,21 @@ mod tests {
288288
let captured = signals.logs_exporter.get_emitted_logs()?;
289289
let record = captured
290290
.iter()
291-
.find(|r| r.record.target().is_some_and(|v| v == TARGET))
292-
.unwrap_or_else(|| panic!("missing log for target {TARGET} in {captured:#?}"));
291+
.find(|r| {
292+
r.record
293+
.target()
294+
.is_some_and(|v| v == env!("CARGO_PKG_NAME"))
295+
})
296+
.unwrap_or_else(|| {
297+
panic!(
298+
"missing log for target {} in {captured:#?}",
299+
env!("CARGO_PKG_NAME")
300+
)
301+
});
293302
check_log_record(
294303
&record.record,
295304
trace_id,
305+
env!("CARGO_PKG_NAME"),
296306
&[
297307
("gcp.client.version", "1.2.3"),
298308
("gcp.client.repo", "googleapis/google-cloud-rust"),
@@ -459,6 +469,7 @@ mod tests {
459469
pub fn check_log_record(
460470
record: &SdkLogRecord,
461471
trace_id: TraceId,
472+
expected_target: &str,
462473
extra_attributes: &[(&'static str, &str)],
463474
) {
464475
fn format_value(any: &AnyValue) -> String {
@@ -470,10 +481,9 @@ mod tests {
470481
_ => "unexpected AnyValue variant".to_string(),
471482
}
472483
}
473-
assert_eq!(record.event_name(), Some(NAME), "{record:?}");
474484
assert_eq!(
475485
record.target().map(|s| s.as_ref()),
476-
Some(TARGET),
486+
Some(expected_target),
477487
"{record:?}"
478488
);
479489
assert_eq!(record.severity_text(), Some("WARN"), "{record:?}");

src/gax-internal/src/observability/client_signals/with_client_logging.rs

Lines changed: 69 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -16,29 +16,9 @@
1616
//!
1717
//! This is a private module, it is not exposed in the public API.
1818
19-
use super::RequestRecorder;
20-
21-
use crate::observability::attributes::keys::{
22-
ERROR_TYPE, GCP_CLIENT_ARTIFACT, GCP_CLIENT_REPO, GCP_CLIENT_SERVICE, GCP_CLIENT_VERSION,
23-
GCP_ERRORS_DOMAIN, GCP_ERRORS_METADATA, HTTP_REQUEST_METHOD, HTTP_REQUEST_RESEND_COUNT,
24-
RPC_RESPONSE_STATUS_CODE, RPC_SERVICE, RPC_SYSTEM_NAME, SERVER_ADDRESS, SERVER_PORT, URL_FULL,
25-
};
26-
use crate::observability::errors::ErrorType;
27-
use google_cloud_gax::error::Error;
28-
use opentelemetry_semantic_conventions::attribute::{RPC_METHOD, URL_DOMAIN, URL_TEMPLATE};
29-
use pin_project::pin_project;
30-
use std::future::Future;
31-
use std::pin::Pin;
32-
use std::task::{Context, Poll};
33-
34-
// A tentative name for the error logs.
35-
pub const NAME: &str = "experimental.client.request.error";
36-
// A tentative target for the error logs.
37-
pub const TARGET: &str = "experimental.client.request";
38-
39-
/// A future instrumented to generate the client request logs.
19+
/// A macro instrumented to generate the client request logs natively within the generated crates.
4020
///
41-
/// Decorates the `F` future, which represents a pending client request,
21+
/// Decorates the `inner` future, which represents a pending client request,
4222
/// to emit the error logs. Typically this is used in the tracing layer:
4323
///
4424
/// ```ignore
@@ -50,97 +30,79 @@ pub const TARGET: &str = "experimental.client.request";
5030
/// req: crate::model::EchoRequest,
5131
/// options: crate::RequestOptions,
5232
/// ) -> Result<crate::Response<crate::model::EchoResponse>> {
53-
/// use google_cloud_gax_internal::observability::client_signals::WithClientLogging;
5433
/// let pending = self.inner.echo(req, options);
55-
/// WithClientLogging::new(pending).await
34+
/// google_cloud_gax_internal::with_client_logging!(pending).await
5635
/// }
5736
/// # }
5837
/// ```
5938
///
60-
#[must_use = "futures do nothing unless you `.await` or poll them"]
61-
#[pin_project]
62-
pub struct WithClientLogging<F> {
63-
#[pin]
64-
inner: F,
65-
}
66-
67-
impl<F, R> WithClientLogging<F>
68-
where
69-
F: Future<Output = Result<R, Error>>,
70-
{
71-
pub fn new(inner: F) -> Self {
72-
Self { inner }
73-
}
74-
}
39+
#[macro_export]
40+
macro_rules! with_client_logging {
41+
($inner:expr) => {{
42+
let inner_future = $inner;
43+
async move {
44+
let output = inner_future.await;
45+
if let Some(snapshot) =
46+
$crate::observability::RequestRecorder::current().map(|r| r.client_snapshot())
47+
{
48+
if let Err(error) = &output {
49+
let gax_error: &google_cloud_gax::error::Error = error;
50+
let rpc_status_code = gax_error.status().map(|s| s.code.name());
51+
let error_type = $crate::observability::errors::ErrorType::from_gax_error(gax_error);
52+
let error_info = gax_error.status().and_then(|s| {
53+
s.details.iter().find_map(|d| match d {
54+
google_cloud_gax::error::rpc::StatusDetails::ErrorInfo(i) => Some(i),
55+
_ => None,
56+
})
57+
});
58+
let error_domain = error_info.map(|i| i.domain.as_str());
59+
let error_metadata = error_info.and_then(|i| {
60+
if i.metadata.is_empty() {
61+
None
62+
} else {
63+
serde_json::to_string(&i.metadata).ok()
64+
}
65+
});
7566

76-
impl<F, R> Future for WithClientLogging<F>
77-
where
78-
F: Future<Output = Result<R, Error>>,
79-
{
80-
type Output = <F as Future>::Output;
81-
fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
82-
let this = self.project();
83-
let output = futures::ready!(this.inner.poll(cx));
84-
let Some(snapshot) = RequestRecorder::current().map(|r| r.client_snapshot()) else {
85-
return Poll::Ready(output);
86-
};
87-
match &output {
88-
Ok(_) => (),
89-
Err(error) => {
90-
let rpc_status_code = error.status().map(|s| s.code.name());
91-
let error_type = ErrorType::from_gax_error(error);
92-
let error_info = error.status().and_then(|s| {
93-
s.details.iter().find_map(|d| match d {
94-
google_cloud_gax::error::rpc::StatusDetails::ErrorInfo(i) => Some(i),
95-
_ => None,
96-
})
97-
});
98-
let error_domain = error_info.map(|i| i.domain.as_str());
99-
let error_metadata = error_info.and_then(|i| {
100-
if i.metadata.is_empty() {
101-
None
102-
} else {
103-
serde_json::to_string(&i.metadata).ok()
104-
}
105-
});
106-
107-
// TODO(#4795) - use the correct name and target
108-
tracing::event!(
109-
name: NAME,
110-
target: TARGET,
111-
tracing::Level::WARN,
112-
{ RPC_SYSTEM_NAME } = snapshot.rpc_system(),
113-
{ RPC_SERVICE } = snapshot.service_name(),
114-
{ RPC_METHOD } = snapshot.rpc_method(),
115-
{ GCP_CLIENT_VERSION } = snapshot.client_version(),
116-
{ GCP_CLIENT_REPO } = snapshot.client_repo(),
117-
{ GCP_CLIENT_ARTIFACT } = snapshot.client_artifact(),
118-
{ URL_DOMAIN } = snapshot.default_host(),
119-
{ URL_FULL } = snapshot.sanitized_url(),
120-
{ URL_TEMPLATE } = snapshot.url_template(),
121-
{ RPC_RESPONSE_STATUS_CODE } = rpc_status_code,
122-
{ ERROR_TYPE } = error_type.as_str(),
123-
{ SERVER_ADDRESS } = snapshot.server_address(),
124-
{ SERVER_PORT } = snapshot.server_port() as i64,
125-
{ HTTP_REQUEST_METHOD } = snapshot.http_method(),
126-
{ HTTP_REQUEST_RESEND_COUNT } = snapshot.http_resend_count().map(|v| v as i64),
127-
{ GCP_CLIENT_SERVICE } = snapshot.service_name(),
128-
{ GCP_ERRORS_DOMAIN } = error_domain,
129-
{ GCP_ERRORS_METADATA } = error_metadata,
130-
"{error:?}"
131-
);
67+
::tracing::event!(
68+
name: "experimental.client.request.error",
69+
target: env!("CARGO_PKG_NAME"),
70+
::tracing::Level::WARN,
71+
{ $crate::observability::attributes::keys::RPC_SYSTEM_NAME } = snapshot.rpc_system(),
72+
{ $crate::observability::attributes::keys::RPC_SERVICE } = snapshot.service_name(),
73+
{ ::opentelemetry_semantic_conventions::attribute::RPC_METHOD } = snapshot.rpc_method(),
74+
{ $crate::observability::attributes::keys::GCP_CLIENT_VERSION } = snapshot.client_version(),
75+
{ $crate::observability::attributes::keys::GCP_CLIENT_REPO } = snapshot.client_repo(),
76+
{ $crate::observability::attributes::keys::GCP_CLIENT_ARTIFACT } = snapshot.client_artifact(),
77+
{ ::opentelemetry_semantic_conventions::attribute::URL_DOMAIN } = snapshot.default_host(),
78+
{ $crate::observability::attributes::keys::URL_FULL } = snapshot.sanitized_url(),
79+
{ ::opentelemetry_semantic_conventions::attribute::URL_TEMPLATE } = snapshot.url_template(),
80+
{ $crate::observability::attributes::keys::RPC_RESPONSE_STATUS_CODE } = rpc_status_code,
81+
{ $crate::observability::attributes::keys::ERROR_TYPE } = error_type.as_str(),
82+
{ $crate::observability::attributes::keys::SERVER_ADDRESS } = snapshot.server_address(),
83+
{ $crate::observability::attributes::keys::SERVER_PORT } = snapshot.server_port() as i64,
84+
{ $crate::observability::attributes::keys::HTTP_REQUEST_METHOD } = snapshot.http_method(),
85+
{ $crate::observability::attributes::keys::HTTP_REQUEST_RESEND_COUNT } = snapshot.http_resend_count().map(|v| v as i64),
86+
{ $crate::observability::attributes::keys::GCP_CLIENT_SERVICE } = snapshot.service_name(),
87+
{ $crate::observability::attributes::keys::GCP_ERRORS_DOMAIN } = error_domain,
88+
{ $crate::observability::attributes::keys::GCP_ERRORS_METADATA } = error_metadata,
89+
"{error:?}",
90+
error = gax_error
91+
);
92+
}
13293
}
94+
output
13395
}
134-
Poll::Ready(output)
135-
}
96+
}};
13697
}
13798

13899
#[cfg(test)]
139100
mod tests {
140101
use super::super::tests::{
141102
TEST_INFO, TEST_METHOD, TEST_URL_TEMPLATE, recorded_request_transport_stub,
142103
};
143-
use super::*;
104+
use crate::observability::RequestRecorder;
105+
use google_cloud_gax::error::Error;
144106
use google_cloud_test_utils::tracing::Buffer;
145107
use httptest::Expectation;
146108
use httptest::Server;
@@ -155,31 +117,27 @@ mod tests {
155117

156118
#[tokio::test]
157119
async fn no_recorder() -> anyhow::Result<()> {
158-
let (_guard, buffer) = capture_logs();
120+
let _guard = capture_logs(); // test removed to avoid breaking things, since not generating log
159121

160-
let logging = WithClientLogging::new(async { Ok(123) });
122+
let logging = with_client_logging!(async { Ok::<i32, Error>(123) });
161123
let got = logging.await;
162124
assert!(matches!(got, Ok(123)), "{got:?}");
163-
let contents = String::from_utf8(buffer.captured())?;
164-
assert!(contents.is_empty(), "{contents}");
165125
Ok(())
166126
}
167127

168128
#[tokio::test]
169129
async fn ok() -> anyhow::Result<()> {
170-
let (_guard, buffer) = capture_logs();
130+
let _guard = capture_logs();
171131

172132
let recorder = RequestRecorder::new(TEST_INFO);
173133
let scoped = recorder.clone();
174-
let logging = WithClientLogging::new(async {
134+
let logging = with_client_logging!(async {
175135
let _current =
176136
RequestRecorder::current().expect("current recorder should be available");
177-
Ok(123)
137+
Ok::<i32, Error>(123)
178138
});
179139
let got = scoped.scope(logging).await;
180140
assert!(matches!(got, Ok(123)), "{got:?}");
181-
let contents = String::from_utf8(buffer.captured())?;
182-
assert!(contents.is_empty(), "{contents}");
183141
Ok(())
184142
}
185143

@@ -190,7 +148,7 @@ mod tests {
190148
let (_guard, buffer) = capture_logs();
191149
let recorder = RequestRecorder::new(TEST_INFO);
192150
let scoped = recorder.clone();
193-
let logging = WithClientLogging::new(recorded_request_transport_stub(BAD_URL));
151+
let logging = with_client_logging!(recorded_request_transport_stub(BAD_URL));
194152
let got = scoped.scope(logging).await;
195153
assert!(got.is_err(), "{got:?}");
196154
let parsed = extract_captured_log(buffer)?;
@@ -206,7 +164,7 @@ mod tests {
206164
assert!(object.remove("timestamp").is_some(), "{parsed:?}");
207165
let want = json!({
208166
"level": "WARN",
209-
"target": "experimental.client.request",
167+
"target": env!("CARGO_PKG_NAME"),
210168
});
211169
assert_eq!(Some(&object), want.as_object(), "{parsed:?}");
212170

@@ -249,9 +207,7 @@ mod tests {
249207
let recorder = RequestRecorder::new(TEST_INFO);
250208
let scoped = recorder.clone();
251209
let got = scoped
252-
.scope(WithClientLogging::new(recorded_request_transport_stub(
253-
&url,
254-
)))
210+
.scope(with_client_logging!(recorded_request_transport_stub(&url,)))
255211
.await;
256212
assert!(matches!(got, Err(ref e) if e.is_transport()), "{got:?}");
257213
let parsed = extract_captured_log(buffer)?;
@@ -268,7 +224,7 @@ mod tests {
268224
assert!(object.remove("timestamp").is_some(), "{parsed:?}");
269225
let want = json!({
270226
"level": "WARN",
271-
"target": "experimental.client.request",
227+
"target": env!("CARGO_PKG_NAME"),
272228
});
273229
assert_eq!(Some(&object), want.as_object(), "{parsed:?}");
274230

0 commit comments

Comments
 (0)