Skip to content

Conversation

HeenaBansal20
Copy link
Contributor

@HeenaBansal20 HeenaBansal20 commented Jul 29, 2025

Fixes open-telemetry/opentelemetry-php#1671
However this change is required for multiple instrumented libraries.

@HeenaBansal20 HeenaBansal20 requested a review from a team as a code owner July 29, 2025 20:56
@HeenaBansal20 HeenaBansal20 marked this pull request as draft July 29, 2025 20:56
Copy link

codecov bot commented Jul 29, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.07%. Comparing base (27b914a) to head (146159c).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##               main     #414   +/-   ##
=========================================
  Coverage     83.07%   83.07%           
  Complexity     1530     1530           
=========================================
  Files            97       97           
  Lines          6115     6115           
=========================================
  Hits           5080     5080           
  Misses         1035     1035           
Flag Coverage Δ
Aws 92.59% <ø> (ø)
Context/Swoole 0.00% <ø> (ø)
Exporter/Instana 49.42% <ø> (ø)
Instrumentation/AwsSdk 81.13% <ø> (ø)
Instrumentation/CakePHP 20.40% <ø> (ø)
Instrumentation/CodeIgniter 73.55% <ø> (ø)
Instrumentation/Curl 90.42% <ø> (ø)
Instrumentation/Doctrine 92.92% <ø> (ø)
Instrumentation/ExtAmqp 88.48% <ø> (ø)
Instrumentation/ExtRdKafka 86.11% <ø> (ø)
Instrumentation/Guzzle 75.58% <ø> (ø)
Instrumentation/HttpAsyncClient 78.04% <ø> (ø)
Instrumentation/IO 70.68% <ø> (ø)
Instrumentation/MongoDB 74.28% <ø> (ø)
Instrumentation/MySqli 95.81% <ø> (ø)
Instrumentation/OpenAIPHP 87.21% <ø> (ø)
Instrumentation/PDO 94.21% <ø> (ø)
Instrumentation/Psr14 76.47% <ø> (ø)
Instrumentation/Psr15 89.15% <ø> (ø)
Instrumentation/Psr16 97.50% <ø> (ø)
Instrumentation/Psr18 77.46% <ø> (ø)
Instrumentation/Psr3 67.01% <ø> (ø)
Instrumentation/Psr6 97.61% <ø> (ø)
Instrumentation/ReactPHP 99.45% <ø> (ø)
Instrumentation/Slim 86.11% <ø> (ø)
Instrumentation/Symfony 84.88% <ø> (ø)
Logs/Monolog 100.00% <ø> (ø)
Propagation/Instana 98.11% <ø> (ø)
Propagation/ServerTiming 100.00% <ø> (ø)
Propagation/TraceResponse 100.00% <ø> (ø)
ResourceDetectors/Azure 91.66% <ø> (ø)
ResourceDetectors/Container 93.02% <ø> (ø)
ResourceDetectors/DigitalOcean 100.00% <ø> (ø)
Sampler/RuleBased 33.51% <ø> (ø)
Shims/OpenTracing 92.45% <ø> (ø)
Utils/Test 87.53% <ø> (ø)

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


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 27b914a...146159c. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@brettmc
Copy link
Contributor

brettmc commented Jul 30, 2025

This looks pretty good to me, is it ready for review?

@HeenaBansal20 HeenaBansal20 marked this pull request as ready for review July 30, 2025 13:50
@HeenaBansal20
Copy link
Contributor Author

@brettmc , yes it is ready for review. DO you want me to open the PRs for the same change for all auto instrumented libraries in separate PR's or in this one only ?

@Soean
Copy link
Contributor

Soean commented Jul 30, 2025

I tested it in my local environment, works for me. ✅

I have also been considering an alternative value. We could have changed the name in the shutdown function. But there is not really a good value in WordPress.

For /example/:

<?php
register_shutdown_function(function () use ($span) {
  
  ...
  
  global $wp;
  echo $wp->query_string; // ‘pagename=example’
  echo $wp->request; // ‘example’
  echo $wp->matched_rule; // ‘(.?.+?)(?:/([0-9]+))?/?$’
  echo $wp->matched_query; // ‘pagename=example&page=’
  
  ...
  
  $span->updateName( ... )`

▶️ We should go with SCRIPT_NAME for now.

@brettmc brettmc merged commit a0d6027 into open-telemetry:main Jul 30, 2025
141 of 153 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.

OLTP span name doesn't follow otel specification

3 participants