Skip to content

Conversation

intuibase
Copy link
Contributor

No description provided.

@intuibase intuibase requested a review from a team as a code owner November 5, 2024 16:28
Copy link

codecov bot commented Nov 5, 2024

Codecov Report

Attention: Patch coverage is 58.57143% with 29 lines in your changes missing coverage. Please review.

Project coverage is 80.44%. Comparing base (2d17302) to head (17509ba).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...rumentation/Symfony/src/SymfonyInstrumentation.php 0.00% 5 Missing ⚠️
src/Context/Swoole/src/SwooleContextStorage.php 0.00% 3 Missing ⚠️
...tion/Laravel/src/Hooks/Illuminate/Queue/Worker.php 0.00% 3 Missing ⚠️
src/Instrumentation/PDO/src/PDOInstrumentation.php 0.00% 3 Missing ⚠️
src/Context/Swoole/src/SwooleContextHandler.php 0.00% 2 Missing ⚠️
...ation/ExtRdKafka/src/ExtRdKafkaInstrumentation.php 50.00% 2 Missing ⚠️
...oks/Illuminate/Foundation/Console/ServeCommand.php 0.00% 2 Missing ⚠️
src/Instrumentation/MySqli/src/MySqliTracker.php 50.00% 2 Missing ⚠️
...c/Instrumentation/Slim/src/SlimInstrumentation.php 0.00% 2 Missing ⚠️
...c/Instrumentation/Curl/src/CurlInstrumentation.php 90.00% 1 Missing ⚠️
... and 4 more
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #315      +/-   ##
============================================
- Coverage     80.67%   80.44%   -0.24%     
- Complexity     1295     1507     +212     
============================================
  Files           114      129      +15     
  Lines          5486     6182     +696     
============================================
+ Hits           4426     4973     +547     
- Misses         1060     1209     +149     
Flag Coverage Δ
Aws 85.55% <100.00%> (?)
Context/Swoole 0.00% <0.00%> (?)
Instrumentation/CakePHP 20.27% <ø> (ø)
Instrumentation/CodeIgniter 73.77% <ø> (ø)
Instrumentation/Curl 90.42% <90.00%> (-0.25%) ⬇️
Instrumentation/ExtAmqp 89.26% <100.00%> (ø)
Instrumentation/ExtRdKafka 86.23% <50.00%> (-1.50%) ⬇️
Instrumentation/Guzzle 69.51% <ø> (ø)
Instrumentation/HttpAsyncClient 78.31% <ø> (ø)
Instrumentation/IO 70.68% <ø> (ø)
Instrumentation/Laravel 62.17% <38.46%> (ø)
Instrumentation/MongoDB 74.28% <100.00%> (+0.24%) ⬆️
Instrumentation/MySqli 95.81% <50.00%> (-0.28%) ⬇️
Instrumentation/OpenAIPHP 87.31% <ø> (ø)
Instrumentation/PDO 90.15% <63.63%> (?)
Instrumentation/Psr14 77.14% <ø> (ø)
Instrumentation/Psr15 89.41% <ø> (ø)
Instrumentation/Psr16 97.56% <ø> (ø)
Instrumentation/Psr18 77.77% <ø> (ø)
Instrumentation/Psr3 59.49% <100.00%> (ø)
Instrumentation/Psr6 97.67% <ø> (ø)
Instrumentation/Slim 86.30% <50.00%> (-0.60%) ⬇️
Instrumentation/Symfony 84.93% <0.00%> (ø)
Instrumentation/Yii 77.68% <ø> (ø)
Logs/Monolog 100.00% <ø> (ø)
Propagation/ServerTiming 100.00% <ø> (ø)
Propagation/TraceResponse 100.00% <ø> (ø)
ResourceDetectors/Azure 91.66% <ø> (ø)
ResourceDetectors/Container 93.02% <ø> (ø)
Sampler/RuleBased 33.51% <ø> (ø)
Shims/OpenTracing 92.45% <ø> (ø)
Symfony 87.81% <ø> (ø)

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

Files with missing lines Coverage Δ
src/Aws/src/AwsSdkInstrumentation.php 48.07% <100.00%> (ø)
src/Aws/src/Ecs/Detector.php 92.38% <100.00%> (ø)
src/Aws/src/Eks/DataProvider.php 100.00% <100.00%> (ø)
src/Aws/src/Xray/IdGenerator.php 100.00% <ø> (ø)
src/Aws/src/Xray/Propagator.php 100.00% <ø> (ø)
...ion/CodeIgniter/src/CodeIgniterInstrumentation.php 78.70% <ø> (ø)
...tion/CodeIgniter/src/ResponsePropagationSetter.php 0.00% <ø> (ø)
...rumentation/ExtAmqp/src/ExtAmqpInstrumentation.php 89.26% <100.00%> (ø)
...strumentation/Guzzle/src/GuzzleInstrumentation.php 74.02% <ø> (ø)
...AsyncClient/src/HttpAsyncClientInstrumentation.php 76.92% <ø> (ø)
... and 52 more

... and 6 files with indirect coverage changes


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 2d17302...17509ba. Read the comment docs.

TraceAttributes::DB_OPERATION => $operationName,
TraceAttributes::DB_NAMESPACE => $query->connection->getDatabaseName(),
TraceAttributes::DB_OPERATION_NAME => $operationName,
/** @phan-suppress-next-line PhanDeprecatedClassConstant */
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea what to do if there is no replacement

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess drop it is the best option

@brettmc
Copy link
Contributor

brettmc commented Nov 7, 2024

These changes look sensible, but I suspect that the semantic convention ones will run into the moratorium on changing produced telemetry.

I don't fully understand the reasoning, but it's becoming a real pain for us: we created a bunch of packages (auto-instrumentation and such) at around semconv 1.24/1.25, and those packages are seemingly stuck on those versions until that moratorium is lifted. But in the meantime, new semconv versions have been released which deprecate some of those semconvs, and our static analysis tools complain.

@brettmc brettmc mentioned this pull request Jan 8, 2025
@intuibase intuibase changed the title Fixed a bit of warnings and deprecations Drop support for PHP 7.4 and 8.0, introduce support for PHP 8.4, fix semconv deprecations Feb 7, 2025
@brettmc
Copy link
Contributor

brettmc commented Feb 8, 2025

Are you happy with this now? I think I have also touched some of the same code in #327 (the build is quite green now), but let's merge this one first and I'll take care of any conflicts in my PR.

@intuibase
Copy link
Contributor Author

@brettmc @ChrisLightfootWild I think PR is ready for your review.
If you will decide to merge, please remove intermediate commit messages - they're not meaningful.

$record = (new LogRecord($log->message))
->setSeverityText($log->level)
->setSeverityNumber(Psr3::severityNumber($log->level))
->setSeverityNumber(Severity::fromPsr3($log->level))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ChrisLightfootWild I think this requires API 1.1.0 and therefore dropping 8.0 support - how does that sound?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I've got the same dependency bumps in #269 so that's fine by me.

Would be good to maybe get this in first and then see if I can update my branch and move that forward a bit.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, will merge!

@brettmc brettmc merged commit 261c2d0 into open-telemetry:main Feb 25, 2025
123 of 124 checks passed
@intuibase intuibase deleted the fix_issues branch February 28, 2025 15:58
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.

3 participants