DataDog / dd-trace-php

Datadog PHP Clients
https://docs.datadoghq.com/tracing/setup/php
Other
501 stars 155 forks source link

#2926: Do not overwrite service in PSR18 integration #2927

Closed TheLevti closed 2 weeks ago

TheLevti commented 2 weeks ago

As described in the issue https://github.com/DataDog/dd-trace-php/issues/2926, for consistency and correctness, do not hardcode the service for this integration otherwise there is no way to control it and it forces that everyone has a service on their service map that is actually just a component.

Description

Reviewer checklist

codecov-commenter commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 7.42%. Comparing base (0e77db0) to head (1c53c86). Report is 1 commits behind head on master.

:exclamation: There is a different number of reports uploaded between BASE (0e77db0) and HEAD (1c53c86). Click for more details.

HEAD has 6 uploads less than BASE | Flag | BASE (0e77db0) | HEAD (1c53c86) | |------|------|------| |tracer-php|12|6|
Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/DataDog/dd-trace-php/pull/2927/graphs/tree.svg?width=650&height=150&src=pr&token=eXio8H7vwF&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog)](https://app.codecov.io/gh/DataDog/dd-trace-php/pull/2927?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog) ```diff @@ Coverage Diff @@ ## master #2927 +/- ## ============================================ - Coverage 82.10% 7.42% -74.68% Complexity 2527 2527 ============================================ Files 108 108 Lines 10359 10358 -1 ============================================ - Hits 8505 769 -7736 - Misses 1854 9589 +7735 ``` | [Flag](https://app.codecov.io/gh/DataDog/dd-trace-php/pull/2927/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog) | Coverage Δ | | |---|---|---| | [tracer-php](https://app.codecov.io/gh/DataDog/dd-trace-php/pull/2927/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog) | `7.42% <ø> (-74.68%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files with missing lines](https://app.codecov.io/gh/DataDog/dd-trace-php/pull/2927?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog) | Coverage Δ | | |---|---|---| | [...rc/DDTrace/Integrations/Psr18/Psr18Integration.php](https://app.codecov.io/gh/DataDog/dd-trace-php/pull/2927?src=pr&el=tree&filepath=src%2FDDTrace%2FIntegrations%2FPsr18%2FPsr18Integration.php&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog#diff-c3JjL0REVHJhY2UvSW50ZWdyYXRpb25zL1BzcjE4L1BzcjE4SW50ZWdyYXRpb24ucGhw) | `0.00% <ø> (-96.56%)` | :arrow_down: | ... and [80 files with indirect coverage changes](https://app.codecov.io/gh/DataDog/dd-trace-php/pull/2927/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog) ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/DataDog/dd-trace-php/pull/2927?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/DataDog/dd-trace-php/pull/2927?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog). Last update [0e77db0...1c53c86](https://app.codecov.io/gh/DataDog/dd-trace-php/pull/2927?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DataDog).
TheLevti commented 2 weeks ago

Hey @TheLevti 👋 Functionally, this makes total sense to me. I'll go ahead and fix the tests if you're ok with that, so that we can merge the PR :shipit:

Thank you very much, yes please.