open-telemetry / open-telemetry/opentelemetry-php
Deprecate Adapter\HttpDiscovery
Nobody has claimed this yet.
- Dominant language
- PHP
- Stars
- 912
- Forks
- 232
- Avg merge
- 7d 16h
- Merged PRs (30d)
- 4
Description
Is your feature request related to a problem?
Redundant code. Currently the OpenTelemetry\SDK\Common\Adapter\HttpDiscovery and OpenTelemetry\SDK\Common\Http\Psr namespaces have overlapping functionality.
Http\Psrhas more features thanAdapter\HttpDiscovery, namely forwarding client options to well-known implementations.- Outside their own namespaces:
Http\Psris used in multiple places, e.g.,PsrTransportFactoryand (ironically?)Adapter\HttpDiscoveryAdapter\HttpDiscoveryis only used inOpenTelemetry\Contrib\Otlp\HttpEndpointResolver, where it could be replaced (see below).
- Overlaps:
Http\Psr\Client\DiscoveryreplacesAdapter\HttpDiscovery\PsrClientResolverdirectlyHttp\Psr\Message\MessageFactorycould easily be expanded with defaults to makeAdapter\HttpDiscovery\MessageFactoryResolverobsolete, without breaking any existing uses ofMessageFactoryAdapter\HttpDiscovery\DependencyResolverandAdapter\HttpDiscovery\HttpPlugClientResolverhave no direct replacements, although their interfaces are inHttp, but I also can't find any instances of them being used.
Describe the solution you'd like
Deprecate Adapter\HttpDiscovery:
- Add the above mentioned functionality to
Http\Psr\Message\MessageFactory - Replace the single existing usage of
Adapter\HttpDiscovery\MessageFactoryResolverwith the expandedMessageFactory - Update all tests
- Add a
@deprecatedPHPDoc tag (or#[Deprecated]attribute >= PHP 8.4) to theAdapter\HttpDiscoveryclasses naming their replacements - Remove the class entirely in
open-telemetry/sdk2.0
Describe alternatives you've considered
I could find no uses of either namespace in opentelemetry-php-contrib. I also searched GitHub for those namespaces and could not find any additional uses outside of the core opentelemetry-php repo and it's forks.
Additional context
I'm working on another contrib component, and didn't want to make a hard-requirement on third-party libraries outside of psr/http-client-implementation and psr/http-factory-implementation. I figured there was likely a shared internal component already built out, and behold, there are two!
I'm happy to take a stab at a PR if the maintainers agree, but I wanted to ask if there was intent or backstory I was unaware of before I spent time on it.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reviewing the Adapter\HttpDiscovery and Http\Psr namespaces, especially the existing use in PsrTransportFactory and OpenTelemetry\Contrib\Otlp\HttpEndpointResolver. Compare the resolver classes and tests, then update the shared MessageFactory, replace the remaining usage, add deprecation documentation with replacements, and ensure the tests pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- backend
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100