Skip to content

TraceResponse and ServerTiming response propagation for CodeIgniter, Yii, Laravel#230

Merged
brettmc merged 1 commit into
open-telemetry:mainfrom
agoallikmaa:response-traceparent-all
Jan 29, 2024
Merged

TraceResponse and ServerTiming response propagation for CodeIgniter, Yii, Laravel#230
brettmc merged 1 commit into
open-telemetry:mainfrom
agoallikmaa:response-traceparent-all

Conversation

@agoallikmaa

Copy link
Copy Markdown
Contributor

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:

  • Symfony already had both
  • CodeIgniter and Yii only had TraceResponse
  • Laravel had neither

Added 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.

@agoallikmaa
agoallikmaa requested a review from a team January 27, 2024 21:48
@codecov

codecov Bot commented Jan 27, 2024

Copy link
Copy Markdown

Codecov Report

Attention: 13 lines in your changes are missing coverage. Please review.

Comparison is base (8f65b2e) 85.65% compared to head (e0cf6d9) 85.38%.

Additional details and impacted files

Impacted file tree graph

@@             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     
Flag Coverage Δ
7.4 85.63% <ø> (ø)
8.0 84.54% <23.52%> (-0.31%) ⬇️
8.1 84.65% <23.52%> (-0.30%) ⬇️
8.2 85.32% <23.52%> (-0.28%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...ion/CodeIgniter/src/CodeIgniterInstrumentation.php 81.37% <33.33%> (-1.46%) ⬇️
src/Instrumentation/Yii/src/YiiInstrumentation.php 86.45% <33.33%> (-1.72%) ⬇️
...nstrumentation/Laravel/src/HttpInstrumentation.php 84.72% <33.33%> (-4.68%) ⬇️
...entation/Laravel/src/ResponsePropagationSetter.php 0.00% <0.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f65b2e...e0cf6d9. Read the comment docs.

@agoallikmaa agoallikmaa changed the title TraceParent and ServerTiming response propagation for CodeIgniter, Yii, Laravel TraceResponse and ServerTiming response propagation for CodeIgniter, Yii, Laravel Jan 27, 2024
Comment thread src/Instrumentation/CodeIgniter/src/CodeIgniterInstrumentation.php
@brettmc
brettmc merged commit 324ecfd into open-telemetry:main Jan 29, 2024
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