Skip to content

Refactor exporter API: setExporters + sampler on interface#5

Merged
loks0n merged 4 commits into
mainfrom
deprecate-add-exporter-add-set-exporters
May 13, 2026
Merged

Refactor exporter API: setExporters + sampler on interface#5
loks0n merged 4 commits into
mainfrom
deprecate-add-exporter-add-set-exporters

Conversation

@loks0n
Copy link
Copy Markdown
Contributor

@loks0n loks0n commented May 13, 2026

Summary

  • Replace Span::addExporter() with variadic Span::setExporters(Exporter ...) (replaces the full set; no-arg call clears).
  • Move sampling onto the Exporter interface via sample(Span): bool. Built-in exporters take an optional sampler closure as their first constructor argument by convention.
  • Sentry is hard-wired to error spans only; a user-supplied sampler is composed (AND) with the error filter, so it can further restrict but not broaden.
  • Span::setStorage() now accepts ?Storage — pass null to clear. resetStorage() and reset() removed.
  • Adds CHANGELOG.md documenting the 3.0.0 breaking changes.

Test plan

  • ./vendor/bin/phpunit — 91 tests pass
  • Verify downstream consumers update bootstrap to setExporters(...) and any setStorage(null) swaps for resetStorage()

🤖 Generated with Claude Code

- Replace Span::addExporter() with variadic Span::setExporters()
- Move sampling onto the Exporter interface (sample(): bool)
- Built-in exporters take an optional sampler closure as first ctor arg
- Sentry composes user sampler with its error-only filter
- Span::setStorage() now accepts ?Storage (null clears); drop resetStorage/reset

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 13, 2026

Greptile Summary

This PR ships the 3.0.0 breaking-change refactor of the exporter API: addExporter / resetExporters / resetStorage / reset are removed in favour of the variadic setExporters(Exporter ...) and a nullable setStorage(?Storage). Sampling responsibility moves from a per-registration closure on Span into an Exporter::sample(Span): bool interface method; built-in exporters accept an optional sampler closure as their first constructor argument.

  • Sentry gains robust DSN validation (empty-string guard + component checks for public key, host, and project ID), and its sampler is AND-composed with the built-in error-only filter so a custom sampler can only narrow, never widen, what is sent.
  • Span.php's finish loop is simplified: it now iterates a plain array<Exporter> and delegates the sample/export decision entirely to each exporter.
  • All 91 tests pass; CHANGELOG and README are fully updated for 3.0.0 consumers.

Confidence Score: 5/5

Safe to merge; the breaking changes are intentional, well-documented, and all tests pass.

The refactor is cleanly scoped: the core Span loop is simpler, the Sentry DSN validation is now robust (empty-string guard + component checks), and the sampling contract is enforced at the interface level. No logic errors were found in the changed paths.

Minor gaps in tests/Exporter/SentryTest.php — Sentry::sample() behavior is not directly tested.

Important Files Changed

Filename Overview
src/Span/Exporter/Sentry.php Adds sampler composition (AND with built-in error filter), empty-DSN guard, and component validation; parameter order changed (sampler first, dsn second with '' default and runtime guard). parse_url===false branch may be dead code in PHP 8.x.
src/Span/Span.php Replaces addExporter/resetExporters/reset/resetStorage with setExporters(variadic) and setStorage(?Storage); sampling loop delegates to Exporter::sample(). Clean refactor.
src/Span/Exporter/Exporter.php Adds sample(Span): bool to the interface; existing export() doc updated. Straightforward interface extension.
tests/Exporter/SentryTest.php Adds tests for empty DSN, missing public key, and missing project ID guards. No coverage for sample() returning false for non-error spans or composed-sampler behavior.
tests/SpanTest.php Migrates all tests to new API (setExporters, setStorage(null)); sampler logic moved into createExporter helper. Coverage is thorough.

Reviews (2): Last reviewed commit: "Apply rector suggestions" | Re-trigger Greptile

Comment thread README.md Outdated
loks0n and others added 3 commits May 13, 2026 11:46
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
parse_url('') returns ['path' => ''] rather than false, so an empty or
incomplete DSN previously slipped through and only failed later as an
opaque cURL error. Reject empty DSNs and DSNs missing the public key,
host, or project ID at construction time.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@loks0n loks0n merged commit cad25b8 into main May 13, 2026
7 checks passed
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