Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.62%. Comparing base (138d1f1) to head (048fc56).
Report is 3 commits behind head on main.

@@            Coverage Diff            @@
##               main     #379   +/-   ##
=========================================
  Coverage     82.61%   82.62%           
- Complexity     1929     1930    +1     
=========================================
  Files           141      141           
  Lines          8063     8067    +4     
=========================================
+ Hits           6661     6665    +4     
  Misses         1402     1402           

I like the PR - OTEL_PHP_INSTRUMENTATION_URL_SANITIZE_FIELD_NAMES makes it sound like this would be available in all instrumentations.

Do you see yourself adding this as a util so instrumentations could share the behavior?

I like the PR - OTEL_PHP_INSTRUMENTATION_URL_SANITIZE_FIELD_NAMES makes it sound like this would be available in all instrumentations.

Do you see yourself adding this as a util so instrumentations could share the behavior?

I would be happy to! Currently, this would add a dependency to the API package (guzzlehttp/psr7), per:

Since that package is licensed MIT, perhaps it is acceptable to add that one class to the OTel-PHP codebase? https://github.com/guzzle/psr7/blob/2.7/src/Query.php

Somewhat related: I plan on adding support for env-based configuration of ConfigProperties which would unify access to declarative config and env-based config and could provide an UriSanitizer implementation that is available to all HTTP instrumentations.

Example configuration provider: InstrumentationConfigurationHttp; the entire package could be moved to contrib once config support is added/moved to an API package
Example instrumentation using the config: AmphpHttpClientInstrumentation

@Nevay are you saying it would be redundant for me to extract my current implementation to a shared utility, and eventually you’ll have something committed that I could replace mine with?

These checks should pass now with #371 merged.