Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Record spans for symfony messenger #215

Merged
merged 14 commits into from
Nov 27, 2023

Conversation

cedricziel
Copy link
Contributor

@cedricziel cedricziel commented Nov 26, 2023

This instrumentation adds spans for the Symfony messenger component.

It creates:

  • an internal span when a message is sent to a MessageBusInterface
  • an internal span, when a message is sent to a transport

Nota bene:

  • enable all testsuites in PHP unit
  • configure Github Actions to only run one job in parallel per PR and cancel existing run when new commits are pushed

Copy link

codecov bot commented Nov 26, 2023

Codecov Report

Merging #215 (48eb722) into main (de8340c) will increase coverage by 0.09%.
The diff coverage is 89.00%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #215      +/-   ##
============================================
+ Coverage     85.85%   85.94%   +0.09%     
- Complexity      883      888       +5     
============================================
  Files            80       81       +1     
  Lines          3393     3493     +100     
============================================
+ Hits           2913     3002      +89     
- Misses          480      491      +11     
Flag Coverage Δ
7.4 85.63% <ø> (ø)
8.0 85.34% <89.00%> (+0.11%) ⬆️
8.1 85.38% <89.00%> (+0.11%) ⬆️
8.2 85.88% <89.00%> (+0.09%) ⬆️

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

Files Coverage Δ
...mentation/Symfony/src/MessengerInstrumentation.php 89.00% <89.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 de8340c...48eb722. Read the comment docs.

@cedricziel
Copy link
Contributor Author

PSR-3 testsuite is broken..

@cedricziel cedricziel marked this pull request as ready for review November 26, 2023 23:05
@cedricziel cedricziel requested a review from a team November 26, 2023 23:05
@brettmc
Copy link
Collaborator

brettmc commented Nov 26, 2023

@cedricziel you can leave the psr3 failures to me, those tests haven't been running previously.

.github/workflows/php.yml Outdated Show resolved Hide resolved
Co-authored-by: Brett McBride <brett.a.mcbride@gmail.com>
@brettmc
Copy link
Collaborator

brettmc commented Nov 27, 2023

The psr-3 failures are sort of xdebug-related. I was able to replicate this locally, and it occurs when running the .phpt tests with coverage enabled, whilst actually generating coverage. eg XDEBUG_MODE=coverage vendor/bin/phpunit --coverage-text
With this command, the output from the phpt test is different (it looks like exporting did not happen, or at least did not make it to console).

So, #217 is a compromise: we run all tests without generating coverage (which includes any phpt tests), and generate coverage only off the unit & integration suites.

The reason this failed to fail previously is that --testsuite unit --testsuite integration is not the right way to run two suites, one clobbers the other. You need to --testsuite unit,integration.

@brettmc
Copy link
Collaborator

brettmc commented Nov 27, 2023

@cedricziel merging in main should make the build green.

@cedricziel
Copy link
Contributor Author

Done @brettmc ! Thanks for the thorough review! 🙏

@brettmc brettmc merged commit 3535662 into open-telemetry:main Nov 27, 2023
67 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.

2 participants