Add utility classes and tests for Timer and Attribute tracking by id …

[GrzegorzDrozd]

…and object

Introduced TimerTrackerById, TimerTrackerByObject, AttributeTrackerById, and AttributeTrackerByObject classes for managing timers and attributes based on id or object references.

This will be used by auto instrumentation and metrics in contrib modules.

[brettmc]

• I'd like to understand how this would be used by an instrumentation package. The metrics implementation is pretty extensive, does it already have features that can be used? I'd prefer to see a metrics implementation PR in an existing package, using this code locally, before extracting it to a re-usable place
• would we have a need for metrics to be tracked across different instrumentation packages?
• instrumentation packages are actively discouraged (per spec) from depending on the SDK

[ChrisLightfootWild]

For any time related operations, there's also already clock functionality in the API - so it might make sense to utilise that over the raw calls to microtime.

[GrzegorzDrozd]

@brettmc Usage example is here: https://github.com/open-telemetry/opentelemetry-php-contrib/pull/391

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[GrzegorzDrozd]

Hi @brettmc , somehow I missed this notification :/

• I'd like to understand how this would be used by an instrumentation package. The metrics implementation is pretty extensive, does it already have features that can be used? I'd prefer to see a metrics implementation PR in an existing package, using this code locally, before extracting it to a re-usable place

For example: TimerTrackerById would keep timers for everything that sending external requests: PDO, Redis, HttpClient, Curl, Guzzle etc. Everything that needs to get duration.

* would we have a need for metrics to be tracked across different instrumentation packages?

Instance of a tracker would be created in ::register of a given contrib package. It would not be shared.

* instrumentation packages are actively discouraged (per spec) from depending on the SDK

Well we can copy and paste those classes all over but that would also be a bad practice.

Maybe some form of cross contrib code share? We could do some namespace shenanigans and have shared code just copied to a contrib module? From this:

  "autoload": {
    "psr-4": {
      "OpenTelemetry\\Contrib\\Instrumentation\\PDO\\": "src/"
    },
    "files": [
      "_register.php"
    ]
  },

to

  "autoload": {
    "psr-4": {
      "OpenTelemetry\\Contrib\\Instrumentation\\PDO\\": "src/",
      "OpenTelemetry\\Contrib\\Instrumentation\\Shared\\": "shared/"
    },
    "files": [
      "_register.php"
    ]
  },

Or even put everything into one file and use files + copy into every contrib?

[brettmc]

Well we can copy and paste those classes all over but that would also be a bad practice.

Agreed, that sounds awful! The most likely place is its own package in contrib, or in our API. If it goes into API, then under API\Instrumentation feels like the right place - we already have some code there to support auto-instrumentation :thinking:

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[codecov[bot]]

Codecov Report

:x: Patch coverage is 94.20290% with 4 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 68.47%. Comparing base (90005d5) to head (5ed6fb3).

Files with missing lines	Patch %	Lines
src/API/Metrics/TimerTrackerById.php	83.33%	2 Missing :warning:
src/API/Metrics/TimerTrackerByObject.php	84.61%	2 Missing :warning:

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1614      +/-   ##
============================================
+ Coverage     68.35%   68.47%   +0.12%     
- Complexity     2970     3001      +31     
============================================
  Files           448      452       +4     
  Lines          8712     8781      +69     
============================================
+ Hits           5955     6013      +58     
- Misses         2757     2768      +11     

Flag	Coverage Δ
8.1	?
8.2	?
8.3	?
8.4	?
8.5	68.47% <94.20%> (+0.18%)	:arrow_up:

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

Files with missing lines	Coverage Δ
...ation/AutoInstrumentation/AttributeTrackerById.php	100.00% <100.00%> (ø)
...n/AutoInstrumentation/AttributeTrackerByObject.php	100.00% <100.00%> (ø)
src/API/Metrics/TimerTrackerById.php	83.33% <83.33%> (ø)
src/API/Metrics/TimerTrackerByObject.php	84.61% <84.61%> (ø)

... and 5 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 90005d5...5ed6fb3. Read the comment docs.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[GrzegorzDrozd]

@brettmc Code is adjusted: files are moved, tests updated etc. I also replaced raw microtime with ClockInterface. I will adjust linked PR to PDO metrics tomorrow.