Remove redundant OTLP XHR-retry trace exporter wrapper - #2599
Open
hahn-kev-bot wants to merge 1 commit into
Open
Remove redundant OTLP XHR-retry trace exporter wrapper#2599hahn-kev-bot wants to merge 1 commit into
hahn-kev-bot wants to merge 1 commit into
Conversation
The OTel dep bump (#2544) took @opentelemetry/otlp-exporter-base from 0.208 to 0.217+, which deleted the browser sendBeacon and XHR transports and now routes all browser OTLP exports through fetch. OTLPTraceExporterBrowserWithXhrRetry existed only to retry a failed sendBeacon export over XHR (forced via a truthy `headers` config, keyed off a "beacon"-worded error). Both the primary and retry exporters now resolve to the fetch transport, and fetch never emits the "beacon" error the retry branch checks for, so the retry path is unreachable. Replace it with a plain browser OTLPTraceExporter and delete the wrapper. Also refresh the stale maxExportBatchSize comment that referenced the old sendBeacon size limit / XHR fallback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 AI summary
Follow-up cleanup to #2598. The OTel dep bump in #2544 took
@opentelemetry/otlp-exporter-basefrom 0.208 to 0.217+ (this branch resolves 0.221), which deleted the browsersendBeaconandXHRtransports — all browser OTLP exports now go throughfetch.createOtlpSendBeaconExportDelegateis now just a deprecated alias forcreateOtlpFetchExportDelegate.OTLPTraceExporterBrowserWithXhrRetryexisted only for the old behavior: on asendBeaconfailure it retried the export with a second exporter built with a truthyheadersconfig to force theXHRtransport, keyed off an error message containing"beacon". After the upgrade both the primary and retry exporters resolve to the fetch transport, and fetch never emits the"beacon"-worded error the retry branch checks for — so the retry path is unreachable and the wrapper adds nothing.Changes:
OTLPTraceExporterBrowserWithXhrRetrywith a plain browserOTLPTraceExporterinotel.client.ts.trace-exporter-browser-with-xhr-retry.ts(no other references).maxExportBatchSizecomment that referenced the oldsendBeaconsize limit / XHR fallback.Verified against the installed/locked package versions (0.217 in the store, 0.221 in the lockfile) that the beacon delegate is a fetch alias and no XHR/beacon transport remains. The #2598 loop fix (
ignoreUrls: [TRACE_EXPORT_URL_PATTERN]on the fetch/XHR instrumentations) is untouched and orthogonal to this cleanup.Test plan
pnpm -C frontend run build✓pnpm -C frontend run lint✓