Skip to content

fix(kafka): [Queue Instrumentation 35] Inject trace headers even without active span#5338

Draft
adinauer wants to merge 3 commits intofeat/queue-instrumentation-otel-messaging-mappingfrom
feat/kafka-producer-wrapper
Draft

fix(kafka): [Queue Instrumentation 35] Inject trace headers even without active span#5338
adinauer wants to merge 3 commits intofeat/queue-instrumentation-otel-messaging-mappingfrom
feat/kafka-producer-wrapper

Conversation

@adinauer
Copy link
Copy Markdown
Member

@adinauer adinauer commented Apr 27, 2026

PR Stack (Queue Instrumentation)


📜 Description

Restructure SentryKafkaProducer.send() to inject trace headers even when no active span exists, and simplify the overall flow to match the SentryFeignClient / SentryOkHttpInterceptor pattern.

Before: When scopes.getSpan() returned null (background workers, @Scheduled jobs, startup publishers), send() short-circuited to the bare delegate — no sentry-trace, baggage, or sentry-task-enqueued-time headers were injected. Distributed tracing silently broke for these common Kafka patterns.

After: Three clean branches matching the Feign/OkHttp/RestTemplate convention:

  • isIgnored → pure delegate (no headers, no span)
  • No active span → inject headers from PropagationContext, no span
  • Active span → start child span, inject headers, delegate with wrapped callback

Also simplifies the implementation:

  • Rename injectHeadersmaybeInjectHeaders with encapsulated try/catch (matches Feign's maybeAddTracingHeaders)
  • Remove outer try/catch around span setup
  • Remove redundant span.isNoOp() early-return branch (wrapping a no-op span callback is harmless)
  • Remove redundant isFinished() guards — Span.finish() is already idempotent via compareAndSet

💡 Motivation and Context

Addresses review finding R10-F001: producer drops trace headers when no active span exists. Background workers and @Scheduled jobs are a primary Kafka producer pattern; losing distributed tracing for them contradicts the Queues product expectations.

💚 How did you test it?

  • Updated existing unit tests to assert headers are injected when no active span exists
  • Updated no-op span test to verify callback wrapping still works
  • Updated header-injection-failure test for new maybeInjectHeaders behavior
  • All 15 SentryKafkaProducerTest tests pass

📝 Checklist

  • I added GH Issue ID & Linear ID
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

Continue addressing remaining review findings from the Queue Instrumentation review.

⚠️ Merge this PR using a merge commit (not squash). Only the collection branch is squash-merged into main.

adinauer and others added 2 commits April 27, 2026 12:39
Replace SentryKafkaProducerInterceptor with SentryKafkaProducer, a
Producer<K,V> wrapper that records a queue.publish span around each
send and finishes it when the broker ack callback fires. The span
now reflects the full async send lifecycle, not just the synchronous
onSend window.

For Spring Boot, the SentryKafkaProducerBeanPostProcessor switches
from patching KafkaTemplate.setProducerInterceptor(...) to installing
a ProducerPostProcessor on every ProducerFactory bean via
ProducerFactory.addPostProcessor(...). KafkaTemplate beans are no
longer touched, so all customer-configured listeners, interceptors
and observation settings are preserved.

The console sample now wraps the raw KafkaProducer instead of setting
INTERCEPTOR_CLASSES_CONFIG. Spring Boot samples need no change — the
auto-configured ProducerPostProcessor is transparent.

Co-Authored-By: Claude <[email protected]>
Decouple header injection from span creation in SentryKafkaProducer
so that distributed tracing works for background workers, @scheduled
jobs, and startup publishers that have no active span.

Restructure send() to match the SentryFeignClient/OkHttp pattern:
- isIgnored: pure delegate, no headers, no span
- No active span: inject headers from PropagationContext, no span
- Active span: start child span, inject headers, wrap callback

Also simplify the implementation:
- Rename injectHeaders to maybeInjectHeaders with encapsulated
  try/catch (matches Feign's maybeAddTracingHeaders pattern)
- Remove outer try/catch around span setup
- Remove redundant span.isNoOp() early-return branch
- Remove redundant isFinished() guards before finish() calls

Co-Authored-By: Claude <[email protected]>
This was referenced Apr 27, 2026
@sentry
Copy link
Copy Markdown

sentry Bot commented Apr 27, 2026

📲 Install Builds

Android

🔗 App Name App ID Version Configuration
SDK Size io.sentry.tests.size 8.37.1 (1) release

⚙️ sentry-android Build Distribution Settings

@github-actions
Copy link
Copy Markdown
Contributor

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 322.31 ms 345.45 ms 23.13 ms
Size 0 B 0 B 0 B

Baseline results on branch: feat/queue-instrumentation-otel-messaging-mapping

Startup times

Revision Plain With Sentry Diff
d5155a4 393.90 ms 465.78 ms 71.88 ms

App size

Revision Plain With Sentry Diff
d5155a4 0 B 0 B 0 B

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant