open-telemetry / open-telemetry/opentelemetry-php

Deprecate Adapter\HttpDiscovery

Open
#1,617 1 comment 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

help wanted
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\Psr has more features than Adapter\HttpDiscovery, namely forwarding client options to well-known implementations.
  • Outside their own namespaces:
    • Http\Psr is used in multiple places, e.g., PsrTransportFactory and (ironically?) Adapter\HttpDiscovery
    • Adapter\HttpDiscovery is only used in OpenTelemetry\Contrib\Otlp\HttpEndpointResolver, where it could be replaced (see below).
  • Overlaps:
    • Http\Psr\Client\Discovery replaces Adapter\HttpDiscovery\PsrClientResolver directly
    • Http\Psr\Message\MessageFactory could easily be expanded with defaults to make Adapter\HttpDiscovery\MessageFactoryResolver obsolete, without breaking any existing uses of MessageFactory
    • Adapter\HttpDiscovery\DependencyResolver and Adapter\HttpDiscovery\HttpPlugClientResolver have no direct replacements, although their interfaces are in Http, 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\MessageFactoryResolver with the expanded MessageFactory
  • Update all tests
  • Add a @deprecated PHPDoc tag (or #[Deprecated] attribute >= PHP 8.4) to the Adapter\HttpDiscovery classes naming their replacements
  • Remove the class entirely in open-telemetry/sdk 2.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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.