search

open-telemetry / opentelemetry-python

OpenTelemetry Python API and SDK
https://opentelemetry.io
Apache License 2.0
1.67k stars 571 forks source link

OLTP Span Exporter isn't compatible with HTTP 2 OLTP endpoints #3833

Open tonybaloney opened 3 months ago

tonybaloney commented 3 months ago

I'm trying to use the OLTP Span exporter for metrics and traces to export to an OLTP endpoint.

This endpoint supports HTTP/2, per the spec OLTP endpoints can support both HTTP/2 and HTTP/1.1 https://opentelemetry.io/docs/specs/otlp/#otlphttp and if they do, they should fall back to HTTP/1.1.

This is my configuration code.

from opentelemetry.sdk.resources import SERVICE_NAME, Resource

from opentelemetry import trace
from opentelemetry.exporter.otlp.proto.http.trace_exporter import OTLPSpanExporter
from opentelemetry.sdk.trace import TracerProvider
from opentelemetry.sdk.trace.export import BatchSpanProcessor

from opentelemetry import metrics
from opentelemetry.exporter.otlp.proto.http.metric_exporter import OTLPMetricExporter
from opentelemetry.sdk.metrics import MeterProvider
from opentelemetry.sdk.metrics.export import PeriodicExportingMetricReader

def configure_oltp_tracing(service_name: str = "demo", endpoint: str = "http://localhost:4317"):
    # Service name is required for most backends
    resource = Resource(attributes={
        SERVICE_NAME: service_name
    })

    traceProvider = TracerProvider(resource=resource)
    processor = BatchSpanProcessor(OTLPSpanExporter(endpoint=f"{endpoint}/v1/traces"))
    traceProvider.add_span_processor(processor)
    trace.set_tracer_provider(traceProvider)

    reader = PeriodicExportingMetricReader(
        OTLPMetricExporter(endpoint=f"{endpoint}/v1/metrics")
    )
    meterProvider = MeterProvider(resource=resource, metric_readers=[reader])
    metrics.set_meter_provider(meterProvider)

I get the following error when traces are exported (and same for metrics)

ERROR:opentelemetry.exporter.otlp.proto.http.trace_exporter:Failed to export batch code: 400, reason: An HTTP/1.x request was sent to an HTTP/2 only endpoint.

The OLTP Exporters for Python are using the requests package, which doesn't support HTTP/2 at all and likely never will since it's only security patches being applied to it nowadays.

Going forward, this package needs to switch to a different HTTP library like httpx to support HTTP/2.

tonybaloney commented 3 months ago

@lzchen and @methane who've both worked on this before

methane commented 3 months ago

First of all, I don't think this is a bug. If client supports both, they must fallback. If they support only one, no fallback is required by the spec.

The OLTP Exporters for Python are using the requests package, which doesn't support HTTP/2 at all and likely never will since it's only security patches being applied to it nowadays.

Although requests is not developed active, they use urllib3. And urllib3 team will support HTTP/2. https://sethmlarson.dev/urllib3-is-fundraising-for-http2-support

There are three ways we can chose.

JamesNK commented 3 months ago

Some more info:

I'm not familiar at all with Python HTTP libraries. Maybe there would need to be an explicit setting to enable pre-negotiated HTTP/2. That's common when it comes to supporting pre-negotiated HTTP/2.

Ousret commented 2 months ago

There's another choice, you could consider Niquests instead. It's a drop-in replacement for Requests and it support HTTP/2 and HTTP/3 by default. Will be happy to assist if needed. The changes required are minimal. And you could easily generate the async interfaces from your sync ones.

The features offered are above others, and you maybe will be particularly interested in the multiplexing.

methane commented 2 months ago

@Ousret it is difficult to change niquests because it conflicts with requests. Note that this library is used by many applications.

This library can not depends on library conflicting with requests. But user may force install Niquests instead of requests in their environemnt.

Ousret commented 2 months ago

How does it conflict? I don’t follow.

Le dim. 21 avr. 2024 à 18:38, Inada Naoki @.***> a écrit :

@Ousret https://github.com/Ousret it is difficult to change niquests because it conflicts with requests. Note that this library is used by many applications.

This library can not depends on library conflicting with requests. But user may force install Niquests instead of requests in their environemnt.

— Reply to this email directly, view it on GitHub https://github.com/open-telemetry/opentelemetry-python/issues/3833#issuecomment-2068128957, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACHFA3DGHOQBPTF5WJRZYL3Y6PTPFAVCNFSM6AAAAABFWMKCGCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRYGEZDQOJVG4 . You are receiving this because you were mentioned.Message ID: @.***>

methane commented 2 months ago 
$ uv pip install requests niquests
Resolved 14 packages in 133ms
Downloaded 2 packages in 29ms
Installed 2 packages in 6ms
 + requests==2.31.0
 + urllib3==2.2.1
inada-n@sonoma:~/niq (main)
$ uv pip uninstall requests urllib3
Uninstalled 2 packages in 27ms
 - requests==2.31.0
 - urllib3==2.2.1
inada-n@sonoma:~/niq (main)
$ .venv/bin/python -c "import niquests"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Users/inada-n/niq/.venv/lib/python3.12/site-packages/niquests/__init__.py", line 48, in <module>
    from ._compat import HAS_LEGACY_URLLIB3
  File "/Users/inada-n/niq/.venv/lib/python3.12/site-packages/niquests/_compat.py", line 25, in <module>
    T = typing.TypeVar("T", urllib3.Timeout, urllib3.Retry)
                            ^^^^^^^^^^^^^^^
AttributeError: module 'urllib3' has no attribute 'Timeout'
Ousret commented 2 months ago

Looks like an issue with uv...

❯ pip install requests niquests                                                                                                                                                                                                                  ─╯
Collecting requests
  Using cached requests-2.31.0-py3-none-any.whl.metadata (4.6 kB)
Collecting niquests
  Using cached niquests-3.6.0-py3-none-any.whl.metadata (13 kB)
Collecting charset-normalizer<4,>=2 (from requests)
  Using cached charset_normalizer-3.3.2-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata (33 kB)
Collecting idna<4,>=2.5 (from requests)
  Using cached idna-3.7-py3-none-any.whl.metadata (9.9 kB)
Collecting urllib3<3,>=1.21.1 (from requests)
  Using cached urllib3-2.2.1-py3-none-any.whl.metadata (6.4 kB)
Collecting certifi>=2017.4.17 (from requests)
  Using cached certifi-2024.2.2-py3-none-any.whl.metadata (2.2 kB)
Collecting kiss-headers<4,>=2 (from niquests)
  Using cached kiss_headers-2.4.3-py3-none-any.whl.metadata (13 kB)
Collecting urllib3-future<3,>=2.7.904 (from niquests)
  Using cached urllib3_future-2.7.904-py3-none-any.whl.metadata (7.0 kB)
Collecting wassima<2,>=1.0.1 (from niquests)
  Using cached wassima-1.1.0-cp37-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata (3.9 kB)
Collecting h11<1.0.0,>=0.11.0 (from urllib3-future<3,>=2.7.904->niquests)
  Using cached h11-0.14.0-py3-none-any.whl.metadata (8.2 kB)
Requirement already satisfied: h2<5.0.0,>=4.0.0 in ./.venv/lib/python3.12/site-packages (from urllib3-future<3,>=2.7.904->niquests) (4.1.0)
Collecting qh3<2.0.0,>=1.0.3 (from urllib3-future<3,>=2.7.904->niquests)
  Using cached qh3-1.0.3-cp37-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata (4.6 kB)
Requirement already satisfied: hyperframe<7,>=6.0 in ./.venv/lib/python3.12/site-packages (from h2<5.0.0,>=4.0.0->urllib3-future<3,>=2.7.904->niquests) (6.0.1)
Requirement already satisfied: hpack<5,>=4.0 in ./.venv/lib/python3.12/site-packages (from h2<5.0.0,>=4.0.0->urllib3-future<3,>=2.7.904->niquests) (4.0.0)
Using cached requests-2.31.0-py3-none-any.whl (62 kB)
Using cached niquests-3.6.0-py3-none-any.whl (114 kB)
Using cached certifi-2024.2.2-py3-none-any.whl (163 kB)
Using cached charset_normalizer-3.3.2-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (141 kB)
Using cached idna-3.7-py3-none-any.whl (66 kB)
Using cached kiss_headers-2.4.3-py3-none-any.whl (43 kB)
Using cached urllib3-2.2.1-py3-none-any.whl (121 kB)
Using cached urllib3_future-2.7.904-py3-none-any.whl (534 kB)
Using cached wassima-1.1.0-cp37-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (998 kB)
Using cached h11-0.14.0-py3-none-any.whl (58 kB)
Using cached qh3-1.0.3-cp37-abi3-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (1.9 MB)
Installing collected packages: wassima, urllib3, qh3, kiss-headers, idna, h11, charset-normalizer, certifi, urllib3-future, requests, niquests
Successfully installed certifi-2024.2.2 charset-normalizer-3.3.2 h11-0.14.0 idna-3.7 kiss-headers-2.4.3 niquests-3.6.0 qh3-1.0.3 requests-2.31.0 urllib3-2.2.1 urllib3-future-2.7.904 wassima-1.1.0

    ~/PycharmProjects/h2    release-5.0 +113 !2 ?7 ··································································································································································· h2   19:26:31  ─╮
❯ python                                                                                                                                                                                                                                         ─╯
Python 3.12.2 (main, Mar 12 2024, 04:24:32) [GCC 13.2.1 20231011 (Red Hat 13.2.1-4)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import requests
>>> import niquests
>>>

work flawlessly with other package manager too.

methane commented 2 months ago

Even if there is no conflicts, I don't want to add dependency to minor library. On application engineer's point of view, more dependency is more container size and more security lisk. Major and mature library is better.

Note that performance is not so important for us because we batch the data before send. And HTTP/2 support is not important for now.

methane commented 2 months ago

Looks like an issue with uv...

You are missing step to reproduce.

$ pip install requests niquests
...
$ pip uninstall requests urllib3  # This is needed!
Uninstalling urllib3-2.2.1:
  Would remove:
    /Users/inada-n/niq/.venv/lib/python3.13/site-packages/urllib3-2.2.1.dist-info/*
    /Users/inada-n/niq/.venv/lib/python3.13/site-packages/urllib3/*
  Would not remove (might be manually added):
    /Users/inada-n/niq/.venv/lib/python3.13/site-packages/urllib3/_async/__init__.py
    /Users/inada-n/niq/.venv/lib/python3.13/site-packages/urllib3/_async/connection.py
    /Users/inada-n/niq/.venv/lib/python3.13/site-packages/urllib3/_async/connectionpool.py
    /Users/inada-n/niq/.venv/lib/python3.13/site-packages/urllib3/_async/poolmanager.py
...
$ .venv/bin/python -c "import niquests"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
    import niquests
  File "/Users/inada-n/niq/.venv/lib/python3.13/site-packages/niquests/__init__.py", line 48, in <module>
    from ._compat import HAS_LEGACY_URLLIB3
  File "/Users/inada-n/niq/.venv/lib/python3.13/site-packages/niquests/_compat.py", line 25, in <module>
    T = typing.TypeVar("T", urllib3.Timeout, urllib3.Retry)
                            ^^^^^^^^^^^^^^^
AttributeError: module 'urllib3' has no attribute 'Timeout'
Ousret commented 2 months ago

Better. But should just have said no, excuses like those aren't receivable from an "engineer" pov. Earth is no longer flat, but was clearly as solid and major at a particular dark time of history. I understand the fear of the unknown.

And HTTP/2 support is not important for now.

It's a decade old protocol. HTTP/1 even more. You should know that its days are counted. More and more service (even if it's slim today) starts to stop serving HTTP/1.

Finally,

pip uninstall requests urllib3

Doing that will most certainly kill a lot of application if not every environment. Your command is surrealist.

Thanks for the answer, still.

methane commented 2 months ago

If we switch from requests to urllib3, users can use urllib3.future instead of urllib3 by just installing urllib3.future, right?