Telemetry support #1529
Replies: 9 comments
-
That's okay.
Possibly, yes. One other aspect of this is that the transport API does give us the option of middleware like approaches as well as the event hooks. Particularly once we have an Eg, we're able to support this sort of thing... class TelemetryTransport:
def __init__(self, *args, **kwargs):
self.transport = httpx.HTTPTransport(**kwargs)
def request(self, *args, **kwargs):
# before request
try:
return self.transport.request(*args, **kwargs)
finally:
# after response Plenty of details to dig into there, but point being that we both have event hooks, and have a way of jumping directly into the control flow. (We'll be able to innovate with more fancy stuff with transport layers too, such as build out a MiddlewareTransport, that does provide a request/response interface. Particularly once we have tightened up the request/response APIs to make them easier to integrate at the transport level.) |
Beta Was this translation helpful? Give feedback.
-
Yeah, I'm digging the use of the transport API for middleware here now. :-) We'd be able to point users at something like this: import httpx
from opentelemetry.instrumentation.httpx import OpenTelemetryTransport
transport = httpx.HTTPTransport()
transport = OpenTelemetryTransport(transport, ...)
with httpx.Client(transport=transport) as client:
... Pretty and simple.
import httpcore
class OpenTelemetryTransport(httpcore.SyncHTTPTransport):
def __init__(self, transport: httpcore.SyncHTTPTransport):
self._transport = transport
self._tracer = ...
def request(self, *args, **kwargs):
with self._tracer.start_as_current_span(...) as span:
... # Store request information on `span`
try:
response = self._transport.request(*args, **kwargs)
except httpcore.RequestError:
... # Process exception
raise
else:
... # Store response information on `span`.
return response
class AsyncOpenTelemetryTransport(httpcore.AsyncHTTPTransport):
def __init__(self, transport: httpcore.AsyncHTTPTransport):
self._transport = transport
self._tracer = ...
async def arequest(self, *args, **kwargs):
with self._tracer.start_as_current_span(...) as span:
... # Store request information on `span`
try:
response = await self._transport.arequest(*args, **kwargs)
except httpcore.RequestError:
... # Process exception
raise
else:
... # Store response information on `span`.
return response (I'm also on the fence wrt encouraging exposing dual transports that implement both sync and async APIs, or sticking to two separate transports. In this case there's no silver bullet — the code has to be written twice in any case. Since we expose the switch in With context-managed requests (encode/httpcore#206) this would look like this: class OpenTelemetryTransport(httpcore.SyncHTTPTransport):
def __init__(self, transport: httpcore.SyncHTTPTransport):
self._transport = transport
self._tracer = ...
def request(self, *args, **kwargs):
with self._tracer.start_as_current_span(...) as span:
... # Store request information on `span`
try:
with self._transport.request(*args, **kwargs) as response:
... # Store response information on `span`.
yield response
except httpcore.RequestError:
... # Process exception
raise That's a very promising way forward because it would be very interoperable. It would make it possible to use the (This begs the question of whether this should be marked under |
Beta Was this translation helpful? Give feedback.
-
Marking this as |
Beta Was this translation helpful? Give feedback.
-
Given the landscape as it currently is, I'd suggest Perhaps at some point there might feasibly be a range of clients it'll be compatible with, but it's the most user-obvious thing at the moment. |
Beta Was this translation helpful? Give feedback.
-
@florimondmanca @tomchristie There are few people asking for the httpx support on contrib repo and on gitter. I am thinking of working on that and came here for prior discussion. It is really nice to see that this has been brought up and discussed. Most of the open-telemetry instrumentation libraries do monkey patching to collect the telemetry data, But If the instrumented library has any built in monitoring support then that is extended to enable telemetry. @florimondmanca has already pointed out one example. Another one similar to it is
|
Beta Was this translation helpful? Give feedback.
-
Anyone looking at this? Currently considering giving |
Beta Was this translation helpful? Give feedback.
-
@lonewolf3739 The answers for 1. and 2. are the same; use a Client instance: transport = httpcore.SyncConnectionPool() # See docs for kwargs, or wait for upcoming `httpx.HTTPTransport()`.
transport = OpenTelemetryTransport(transport)
with httpx.Client(transport=transport) as client:
r = client.get('https://www.example.org/') Then you can pass the client around in the context, or like this: TelemetryClient = httpx.Client(transport=transport)
...
with TelemetryClient as client:
r = client.get('https://www.example.org/') Once httpx 0.17.0 gets cut and released, you can use |
Beta Was this translation helpful? Give feedback.
-
Generally the top-level API is meant for very simple cases, or prototyping. I can understand there will be cases when this is all users need in production even, but I'd also insist on the idea that it might be okay to ask users to use a client just to benefit from telemetry. "Use the client" is not an unusual recommendation we make to users anyway for advanced use cases. And it reduces the burden on both opentelemetry and HTTPX authors, so seems like a net positive for all. :-) |
Beta Was this translation helpful? Give feedback.
-
So, I've got a use-case which is helping illuminate some requirements here. Working with the HTTPX command line client the Really there's a whole slew of additional points of information that we'd like to be able to hook into in order to have a CLI that's able to fully reflect the request/response cycle to the user.
I don't necessarily think we want this to be exposed via our existing event hooks. It's a different, detailed level, that's about debugging inside the network layer. It might be that we can expose it all the way out to the event hooks, but let's not start from that POV. At a basic level this is some functionality that we want to figure out as a Transport API extension. transport.handle_request(..., extensions={"hooks": ...}) We need to be able to tie sets of callbacks together as each belonging to the same request. My feeling is that we ought to keep things simple by having the responsibility for sequencing requests be with the caller, rather than having the transport track and increment request IDs. For example, this kind of thing... def on_callback(message, context):
...
transport.handle_request(
method=b"GET",
url=(b"https", b"www.example.com", None, b"/"),
headers=[(b"Host", b"www.example.com")],
stream=httpx.ByteStream(b""),
extensions={
"hooks": {"callback": on_callback, "context": {"request_id": 1}}
}
) Design questions:
|
Beta Was this translation helpful? Give feedback.
-
Prompted by discussions on Gitter, and somewhat related to #790.
"Telemetry" refers to the possibility of extracting metrics from a running program that uses HTTPX, in order to get insights on performed requests (what they are, how many of them, to which endpoints, etc).
So…
What do other HTTP clients do?
Requests
Requests doesn't have any telemetry built-in, nor any API specifically targeted at telemetry/tracing. (Hooks don't seem to be enough, since there's only a
response
hook.)For this reason it seems most instrumentation projects like OpenTelemetry and Datadog APM take a monkeypatching approach.
For example, OpenTelemetry monkeypatches the high-level API (and nothing else, which means session requests aren't traced):
And Datadog APM monkeypatches
Session.send()
(which also has limitations because then custom/subclassed sessions aren't monkeypatched by default):aiohttp
aiohttp
has a Client Tracing API, which is basically "event hooks on steroids". The Tracing Reference lists 15 different callbacks, most of them related to HTTP networking rather than client functionality.OpenTelemetry integrates with this feature by exporting a
create_trace_config()
function, as well as a helper for preparing span names:Datadog APM doesn't have support for
aiohttp
yet (but it does support OpenTelemetry in general, for the above setup + the OpenTelemetry Datadog exporter would do the trick).OpenTelemetry
As mentioned by @agronholm on Gitter, the current trend seems to shift library integration support to OpenTelemetry, a CNCF project for collecting and exposing metrics from applications. The opentelemetry-python client has support for a variety of exporters (Prometheus (an open standard for metric ingestion as well), Datadog, OpenCensus…), as well as support for instrumentating various libraries. (There's even instrumentation for Starlette and ASGI — how interesting! I need to add this to awesome-asgi…)
Thanks to the exporter mechanism, vendors can integrate with OpenTelemetry, removing the need for building and maintaining vendor-specific integrations.
From my perspective, we'll most likely want HTTPX to be supported by OpenTelemetry, since by transitivity this will give us the widest vendor support possible.
(Side note: I checked, and OpenTelemetry seems to support
asyncio
fine enough via itsRuntimeContext
API (although it's sync on the surface) - see open-telemetry/opentelemetry-python#395. Doesn't seem like there's any support for trio or curio, though.)How?
Update 2020-11-23: This is outdated — we'll most likely want to build on top of the Transport API, see #1264 (comment).
A minimal HTTPX-OpenTelemetry integration could maybe (see questions/blockers below) be built on the upcoming Event Hooks functionality (#790, #1215).
Taking inspiration from the
aiohttp
OpenTelemetry implementation, I ended up with a proof-of-concept implementation in this gist:https://gist.github.com/florimondmanca/37ffea4dc4aac0bad8f30534783b0b3d
Would be used like so:
There are a couple of questions/blockers, though:
request
hook to theresponse
hook. Right now the gist does this naively by storing an attribute on therequest
, but this would break down completely if any component in the call stack swaps the request for another (eg authentication or redirects).request_exception
hook? Theaiohttp
integration uses such a hook to store an error status code based on exception information.Overall, it feels like implementing this on top of a middleware-like API (rather than callback-based event hooks) would allow addressing these issues while providing an elegant implementation:
Such an "interceptor" API (as described in #790 (comment)) would also make the usage simpler, since we don't need to unpack a 2-tuple of callables and put them in the correct dict entries…
Beta Was this translation helpful? Give feedback.
All reactions