impl(o11y-tracing): add opentelemetry dependencies and HeaderInjector#4980

Merged

westarle merged 7 commits intogoogleapis:mainfrom

westarle:trace-propagation-deps

Mar 12, 2026

Contributor

westarle commented Mar 10, 2026

No description provided.

westarle force-pushed the trace-propagation-deps branch from 5649b5d to df4f4cd Compare

March 10, 2026 20:46

codecov bot commented Mar 10, 2026 •

edited

Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.81%. Comparing base (40581e2) to head (d9e8e84).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4980   +/-   ##
=======================================
  Coverage   92.80%   92.81%           
=======================================
  Files         224      225    +1     
  Lines        8656     8665    +9     
=======================================
+ Hits         8033     8042    +9     
  Misses        623      623

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

westarle force-pushed the trace-propagation-deps branch 4 times, most recently from 230a7cc to 060718e Compare

March 11, 2026 15:06


          feat(gax-internal): add opentelemetry dependencies and HeaderInjector

45f4e2d

westarle force-pushed the trace-propagation-deps branch 3 times, most recently from ecf1086 to 378d2dd Compare

March 12, 2026 18:29


          docs: update RESEARCH.md to clarify localized propagator reasoning

4153ca6

westarle force-pushed the trace-propagation-deps branch from 378d2dd to 4153ca6 Compare

March 12, 2026 19:12

westarle marked this pull request as ready for review

March 12, 2026 19:37

westarle requested a review from a team as a code owner

March 12, 2026 19:37


          Merge branch 'main' into trace-propagation-deps

14fc73f

westarle requested a review from haphungw

March 12, 2026 19:38

coryan requested changes

View reviewed changes

src/gax-internal/src/observability/propagation.rs Outdated

+              impl<'a> opentelemetry::propagation::Injector for HeaderInjector<'a> {
+                  fn set(&mut self, key: &str, value: String) {
+                      if let Ok(name) = http::header::HeaderName::from_bytes(key.as_bytes()) {

Collaborator

coryan Mar 12, 2026

Move to the module as use http::header::HeaderName.

Contributor Author

westarle Mar 12, 2026

Done

src/gax-internal/src/observability/propagation.rs Outdated

+              impl<'a> opentelemetry::propagation::Injector for HeaderInjector<'a> {
+                  fn set(&mut self, key: &str, value: String) {
+                      if let Ok(name) = http::header::HeaderName::from_bytes(key.as_bytes()) {

Collaborator

coryan Mar 12, 2026

Why not from_str():

https://docs.rs/http/latest/http/header/struct.HeaderName.html#impl-FromStr-for-HeaderName

Contributor Author

westarle Mar 12, 2026

Thanks, done!

src/gax-internal/src/observability/propagation.rs Outdated

+                  // If the tracing span doesn't have a valid trace ID (e.g., the user isn't
+                  // using the tracing_opentelemetry subscriber bridge), fall back to the global
+                  // OTel context (for pure opentelemetry_sdk users).
+                  use opentelemetry::trace::TraceContextExt;

Collaborator

coryan Mar 12, 2026

move to the module

Contributor Author

westarle Mar 12, 2026

done

src/gax-internal/src/observability/propagation.rs Outdated

+                  // OTel context (for pure opentelemetry_sdk users).
+                  use opentelemetry::trace::TraceContextExt;
+                  if !context.span().span_context().is_valid() {
+                      context = opentelemetry::Context::current();

Collaborator

coryan Mar 12, 2026

add a use declaration at the module level.

Contributor Author

westarle Mar 12, 2026

done

src/gax-internal/src/observability/propagation.rs Outdated

Comment on lines +53 to +54

		let propagator = opentelemetry_sdk::propagation::TraceContextPropagator::new();
		use opentelemetry::propagation::TextMapPropagator;

Collaborator

coryan Mar 12, 2026

After several refactors I convinced myself that use declarations work better in practice that fully qualified names. And the Google Style Guide recommends them too (if that convinces you).

src/gax-internal/src/observability/propagation.rs Show resolved Hide resolved

src/gax-internal/src/observability/propagation.rs Outdated

Comment on lines +133 to +140

+                      // 4. Verify the traceparent header was successfully injected
+                      assert!(
+                          headers.contains_key("traceparent"),
+                          "Headers: {:?}",
+                          headers
+                      );
+                      let traceparent = headers.get("traceparent").unwrap().to_str().unwrap();
+                      assert!(traceparent.starts_with("00-"));

Collaborator

coryan Mar 12, 2026

see below, these assertions can be simplified with matches!()

Contributor Author

westarle Mar 12, 2026

done

src/gax-internal/src/observability/propagation.rs Outdated

Comment on lines +110 to +120

+                  #[test]
+                  fn test_inject_context_success() {
+                      // 1. Setup tracing with OpenTelemetry layer
+                      let tracer_provider = opentelemetry_sdk::trace::SdkTracerProvider::builder().build();
+                      use opentelemetry::trace::TracerProvider as _;
+                      let tracer = tracer_provider.tracer("test");
+                      use tracing_subscriber::layer::SubscriberExt;
+                      let telemetry = tracing_opentelemetry::layer().with_tracer(tracer);
+                      let subscriber = tracing_subscriber::registry().with(telemetry);
+                      let dispatcher = tracing::Dispatch::new(subscriber);

Collaborator

coryan Mar 12, 2026

You can save yourself explanations with a helper function:

Suggested change

      
                #[test]
          
                fn test_inject_context_success() {
          
                    // 1. Setup tracing with OpenTelemetry layer
          
                    let tracer_provider = opentelemetry_sdk::trace::SdkTracerProvider::builder().build();
          
                    use opentelemetry::trace::TracerProvider as _;
          
                    let tracer = tracer_provider.tracer("test");
          
                    use tracing_subscriber::layer::SubscriberExt;
          
                    let telemetry = tracing_opentelemetry::layer().with_tracer(tracer);
          
                    let subscriber = tracing_subscriber::registry().with(telemetry);
          
                    let dispatcher = tracing::Dispatch::new(subscriber);
          
                fn set_up_otel_and_tracing() -> Dispatch {
          
                    let tracer_provider = opentelemetry_sdk::trace::SdkTracerProvider::builder().build();
          
                    use opentelemetry::trace::TracerProvider as _;
          
                    let tracer = tracer_provider.tracer("test");
          
                    use tracing_subscriber::layer::SubscriberExt;
          
                    let telemetry = tracing_opentelemetry::layer().with_tracer(tracer);
          
                    let subscriber = tracing_subscriber::registry().with(telemetry);
          
                    tracing::Dispatch::new(subscriber)
          
                }
          
                #[test]
          
                fn inject_context_success() {
          
                    let dispatch = set_up_otel_and_tracing();

Contributor Author

westarle Mar 12, 2026

done, this is much better.

src/gax-internal/src/observability/propagation.rs Outdated

+                  use opentelemetry::propagation::Injector;
+                  #[test]
+                  fn test_injector_valid_headers() {

Collaborator

coryan Mar 12, 2026

The test_ prefix is not idiomatic in Rust.

Contributor Author

westarle Mar 12, 2026

done

src/gax-internal/src/observability/propagation.rs Outdated

+              impl<'a> opentelemetry::propagation::Injector for HeaderInjector<'a> {
+                  fn set(&mut self, key: &str, value: String) {
+                      if let Ok(name) = http::header::HeaderName::from_bytes(key.as_bytes()) {

Collaborator

coryan Mar 12, 2026

I think you can write:

Suggested change

      
                    if let Ok(name) = http::header::HeaderName::from_bytes(key.as_bytes()) {
          
                    if let (Ok(key), Ok(value)) = (HeaderName::from_str(key), HeaderValue::from_str(&value)) {

Contributor Author

westarle Mar 12, 2026

Done, looks much better this way!

westarle added 3 commits

March 12, 2026 17:54


          fix(gax-internal): address PR feedback for HeaderInjector

6efea4a


          refactor(gax-internal): use module-level imports for trace propagation

dcc8440


          Merge branch 'main' into trace-propagation-deps

861b222

coryan approved these changes

View reviewed changes

src/gax-internal/src/observability/propagation.rs Outdated

+                      // Invalid characters in header key
+                      injector.set("invalid key\n", "value".to_string());
+                      assert!(headers.is_empty());

Collaborator

coryan Mar 12, 2026

Suggested change

      
                    assert!(headers.is_empty());
          
                    assert!(headers.is_empty(), "{headers:?}");

In general, assert!(expr); is typically too little context to debug things.


          test(gax-internal): add debug context to assertions

d9e8e84

westarle enabled auto-merge (squash)

March 12, 2026 22:03

westarle merged commit 4e48dac into googleapis:main

35 checks passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet