Skip to content

feat: re-enable Telemetry Processor and simplify setup#1254

Open
giortzisg wants to merge 5 commits intomasterfrom
feat/enable-telem
Open

feat: re-enable Telemetry Processor and simplify setup#1254
giortzisg wants to merge 5 commits intomasterfrom
feat/enable-telem

Conversation

@giortzisg
Copy link
Copy Markdown
Contributor

@giortzisg giortzisg commented Apr 8, 2026

Description

This PR re-enables support for the Telemetry Processor and includes simplifying it's setup.

  • Switch client-side DSN storage and parsing from the public sentry.Dsn wrapper to internal/protocol.Dsn during client initialization.
  • Update envelope headers and transport code to carry *protocol.Dsn directly instead of converting back and forth to strings.
  • Move the telemetry processor setup decision into NewClient, so client startup chooses one path up front:
    • use the telemetry processor when telemetry buffering is enabled and no custom transport is provided
    • otherwise fall back to the legacy transport plus the existing batch log/metric processors
  • Remove now-redundant guard logic from setupTelemetryProcessor, since the caller is now responsible for deciding whether that path is valid.
  • Update tests to reflect the new empty-DSN behavior, where the telemetry processor is still initialized and backed by the internal noop transport.
  • Make the client reports integration test wait for asynchronously delivered reports instead of assuming they are immediately present.

Issues

Changelog Entry Instructions

To add a custom changelog entry, uncomment the section above. Supports:

  • Single entry: just write text
  • Multiple entries: use bullet points
  • Nested bullets: indent 4+ spaces

For more details: custom changelog entries

Reminders

Changelog Entry

  • Re-enable Telemetry Processor by default. To disable the behavior use the DisableTelemetryBuffer flag.
  • Simplify client DSN storage to internal/protocol.Dsn and make it safe to access.

@giortzisg giortzisg self-assigned this Apr 8, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

Semver Impact of This PR

🟡 Minor (new features)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


New Features ✨

  • Re-enable Telemetry Processor by default. To disable the behavior use the DisableTelemetryBuffer flag. by giortzisg in #1254
  • Simplify client DSN storage to internal/protocol.Dsn and make it safe to access. by giortzisg in #1254

Internal Changes 🔧

Deps

  • Bump go.opentelemetry.io/otel/sdk from 1.40.0 to 1.43.0 in /otel by dependabot in #1256
  • Bump go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp from 1.40.0 to 1.43.0 in /otel/otlp by dependabot in #1255

🤖 This preview updates automatically when you update the PR.

giortzisg added a commit to getsentry/gib-potato that referenced this pull request Apr 8, 2026
this change pins getsentry/sentry-go#1254, in
order to test the Telemetry Processor
@giortzisg giortzisg force-pushed the feat/enable-telem branch from ab648d1 to f12e2cd Compare April 9, 2026 10:10
Copy link
Copy Markdown

@Litarnus Litarnus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

the TestClientReports_Integration was racing with the buffer when the
event was added. We should use requireEventually to wait for the event
to first be added and then Flush
conditionally having a nil telemetryProcessor creates some conditional
issues and is painful to debug. Disabling it when a nil dsn arrives is a
tiny optimization that didn't make sense, thus we always should create
one when `NewClient` is called.
@giortzisg giortzisg force-pushed the feat/enable-telem branch from d0e5d15 to 5229042 Compare April 9, 2026 13:52
@giortzisg giortzisg changed the title feat: re-enable Telemetry Processor feat: re-enable Telemetry Processor and simplify setup Apr 9, 2026
@giortzisg giortzisg marked this pull request as ready for review April 15, 2026 08:29
Comment thread client.go
Comment thread client.go
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 3d172e2. Configure here.

Comment thread client.go
reportRecorder: report.NoopRecorder(),
reportProvider: report.NoopProvider(),
}
client.setupIntegrations()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved setupIntegrations before Transport initialization breaks custom integrations

Medium Severity

setupIntegrations() was moved from after transport setup to before it. Custom integrations provided via options.Integrations that access client.Transport in their SetupOnce callback will now get a nil Transport, which was previously always set. This is a behavioral regression since client.Transport is a public field that integrations could reasonably access during setup.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3d172e2. Configure here.

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.

2 participants