TraceResponse and ServerTiming response propagation for CodeIgniter, Yii, Laravel#230
Merged
brettmc merged 1 commit intoJan 29, 2024
Merged
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #230 +/- ##
============================================
- Coverage 85.65% 85.38% -0.28%
- Complexity 958 964 +6
============================================
Files 86 87 +1
Lines 3883 3900 +17
============================================
+ Hits 3326 3330 +4
- Misses 557 570 +13
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
brettmc
reviewed
Jan 27, 2024
brettmc
approved these changes
Jan 28, 2024
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.
There are 2 response propagators which include a W3C TraceParent formatted tracing context info: TraceResponse and ServerTiming. The latter is necessary for RUM integration since frontend JS can read the value from ServerTiming, and the former is the more standardized method, but unsuitable for RUM until browsers add it to the list of headers that can be read from JS.
Previous state:
TraceResponseAdded the missing ones to make sure both are present for Symfony, Laravel, CodeIgniter and Yii. PSR-15 and consequently Slim have a method for setting headers that isn't trivially compatible with
PropagationSetterInterface(as response is modified by$response = $response->withX(...)pattern), so it is not included here.As integration tests are not applicable for testing that the headers actually get set, tested this manually for each of them.
The code duplication for each framework for this purpose is not ideal, so at some point we should think of ways to fix that, but it is not in the scope for this PR.