Skip to content

Commit

Permalink
Allow instrumenting a single httpx client, fix tests for OTel 1.28 (#575
Browse files Browse the repository at this point in the history
)
  • Loading branch information
alexmojaki authored Nov 12, 2024
1 parent 15ff969 commit d8042bf
Show file tree
Hide file tree
Showing 11 changed files with 581 additions and 641 deletions.
29 changes: 20 additions & 9 deletions logfire/_internal/integrations/httpx.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from __future__ import annotations

from typing import TYPE_CHECKING
from typing import TYPE_CHECKING, Any

from opentelemetry.instrumentation.httpx import HTTPXClientInstrumentor

Expand All @@ -25,15 +25,26 @@ class HTTPXInstrumentKwargs(TypedDict, total=False):
skip_dep_check: bool


def instrument_httpx(logfire_instance: Logfire, **kwargs: Unpack[HTTPXInstrumentKwargs]) -> None:
def instrument_httpx(
logfire_instance: Logfire, client: httpx.Client | httpx.AsyncClient | None, **kwargs: Unpack[HTTPXInstrumentKwargs]
) -> None:
"""Instrument the `httpx` module so that spans are automatically created for each request.
See the `Logfire.instrument_httpx` method for details.
"""
HTTPXClientInstrumentor().instrument( # type: ignore[reportUnknownMemberType]
**{
'tracer_provider': logfire_instance.config.get_tracer_provider(),
'meter_provider': logfire_instance.config.get_meter_provider(),
**kwargs,
}
)
final_kwargs: dict[str, Any] = {
'tracer_provider': logfire_instance.config.get_tracer_provider(),
'meter_provider': logfire_instance.config.get_meter_provider(),
**kwargs,
}
del kwargs # make sure only final_kwargs is used
instrumentor = HTTPXClientInstrumentor()
if client:
instrumentor.instrument_client(
client,
tracer_provider=final_kwargs['tracer_provider'],
request_hook=final_kwargs.get('request_hook'),
response_hook=final_kwargs.get('response_hook'),
)
else:
instrumentor.instrument(**final_kwargs) # type: ignore[reportUnknownMemberType]
9 changes: 7 additions & 2 deletions logfire/_internal/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
from wsgiref.types import WSGIApplication

import anthropic
import httpx
import openai
from django.http import HttpRequest, HttpResponse
from fastapi import FastAPI
Expand Down Expand Up @@ -1084,17 +1085,21 @@ def instrument_asyncpg(self, **kwargs: Unpack[AsyncPGInstrumentKwargs]) -> None:
self._warn_if_not_initialized_for_instrumentation()
return instrument_asyncpg(self, **kwargs)

def instrument_httpx(self, **kwargs: Unpack[HTTPXInstrumentKwargs]) -> None:
def instrument_httpx(
self, client: httpx.Client | httpx.AsyncClient | None = None, **kwargs: Unpack[HTTPXInstrumentKwargs]
) -> None:
"""Instrument the `httpx` module so that spans are automatically created for each request.
Optionally, pass an `httpx.Client` instance to instrument only that client.
Uses the
[OpenTelemetry HTTPX Instrumentation](https://opentelemetry-python-contrib.readthedocs.io/en/latest/instrumentation/httpx/httpx.html)
library, specifically `HTTPXClientInstrumentor().instrument()`, to which it passes `**kwargs`.
"""
from .integrations.httpx import instrument_httpx

self._warn_if_not_initialized_for_instrumentation()
return instrument_httpx(self, **kwargs)
return instrument_httpx(self, client, **kwargs)

def instrument_celery(self, **kwargs: Unpack[CeleryInstrumentKwargs]) -> None:
"""Instrument `celery` so that spans are automatically created for each task.
Expand Down
13 changes: 9 additions & 4 deletions logfire/_internal/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from typing import Any, Generic, Sequence, TypeVar
from weakref import WeakSet

from opentelemetry.context import Context
from opentelemetry.metrics import (
CallbackT,
Counter,
Expand Down Expand Up @@ -254,8 +255,9 @@ def add(
self,
amount: int | float,
attributes: Attributes | None = None,
context: Context | None = None,
) -> None:
self._instrument.add(amount, attributes)
self._instrument.add(amount, attributes, context)

def _create_real_instrument(self, meter: Meter) -> Counter:
return meter.create_counter(self._name, self._unit, self._description)
Expand All @@ -266,8 +268,9 @@ def record(
self,
amount: int | float,
attributes: Attributes | None = None,
context: Context | None = None,
) -> None:
self._instrument.record(amount, attributes)
self._instrument.record(amount, attributes, context)

def _create_real_instrument(self, meter: Meter) -> Histogram:
return meter.create_histogram(self._name, self._unit, self._description)
Expand Down Expand Up @@ -299,8 +302,9 @@ def add(
self,
amount: int | float,
attributes: Attributes | None = None,
context: Context | None = None,
) -> None:
self._instrument.add(amount, attributes)
self._instrument.add(amount, attributes, context)

def _create_real_instrument(self, meter: Meter) -> UpDownCounter:
return meter.create_up_down_counter(self._name, self._unit, self._description)
Expand All @@ -313,8 +317,9 @@ def set(
self,
amount: int | float,
attributes: Attributes | None = None,
context: Context | None = None,
) -> None: # pragma: no cover
self._instrument.set(amount, attributes)
self._instrument.set(amount, attributes, context)

def _create_real_instrument(self, meter: Meter): # pragma: no cover
return meter.create_gauge(self._name, self._unit, self._description)
Expand Down
4 changes: 2 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ build-backend = "hatchling.build"
[project]
name = "logfire"
version = "2.1.2"
description = "The best Python observability tool! 🪵🔥"
description = "The best Python observability tool!"
requires-python = ">=3.8"
authors = [
{ name = "Pydantic Team", email = "engineering@pydantic.dev" },
Expand Down Expand Up @@ -147,9 +147,9 @@ dev = [
"numpy<1.24.4; python_version < '3.9'",
"pytest-recording>=0.13.2",
"uvicorn>=0.30.6",
# Logfire API
"logfire-api",
"requests",
"setuptools>=75.3.0",
]
docs = [
"mkdocs>=1.5.0",
Expand Down
5 changes: 4 additions & 1 deletion tests/otel_integrations/test_django.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ def test_good_route(client: Client, exporter: TestExporter, metrics_reader: InMe
'start_time_unix_nano': IsInt(),
'time_unix_nano': IsInt(),
'value': 0,
'exemplars': [],
}
],
'aggregation_temporality': 2,
Expand All @@ -52,7 +53,7 @@ def test_good_route(client: Client, exporter: TestExporter, metrics_reader: InMe
},
{
'name': 'http.server.duration',
'description': 'Duration of HTTP server requests.',
'description': 'Measures the duration of inbound HTTP requests.',
'unit': 'ms',
'data': {
'data_points': [
Expand All @@ -77,6 +78,7 @@ def test_good_route(client: Client, exporter: TestExporter, metrics_reader: InMe
'flags': 0,
'min': IsNumeric(),
'max': IsNumeric(),
'exemplars': [],
}
],
'aggregation_temporality': 1,
Expand Down Expand Up @@ -107,6 +109,7 @@ def test_good_route(client: Client, exporter: TestExporter, metrics_reader: InMe
'flags': 0,
'min': IsNumeric(),
'max': IsNumeric(),
'exemplars': [],
}
],
'aggregation_temporality': 1,
Expand Down
15 changes: 5 additions & 10 deletions tests/otel_integrations/test_httpx.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,6 @@
from logfire.testing import TestExporter


@pytest.fixture(autouse=True) # only applies within this module
def instrument_httpx():
logfire.instrument_httpx()
try:
yield
finally:
HTTPXClientInstrumentor().uninstrument() # type: ignore


@pytest.mark.anyio
async def test_httpx_instrumentation(exporter: TestExporter):
# The purpose of this mock transport is to ensure that the traceparent header is provided
Expand All @@ -30,7 +21,11 @@ def handler(request: Request):
assert span.context
trace_id = span.context.trace_id
with httpx.Client(transport=transport) as client:
response = client.get('https://example.org/')
logfire.instrument_httpx(client)
try:
response = client.get('https://example.org/')
finally:
HTTPXClientInstrumentor().uninstrument() # type: ignore
# Validation of context propagation: ensure that the traceparent header contains the trace ID
traceparent_header = response.headers['traceparent']
assert f'{trace_id:032x}' == traceparent_header.split('-')[1]
Expand Down
4 changes: 2 additions & 2 deletions tests/otel_integrations/test_openai.py
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,7 @@ def test_images(instrumented_client: openai.Client, exporter: TestExporter) -> N

def test_dont_suppress_httpx(exporter: TestExporter) -> None:
with httpx.Client(transport=MockTransport(request_handler)) as httpx_client:
HTTPXClientInstrumentor.instrument_client(httpx_client)
HTTPXClientInstrumentor().instrument_client(httpx_client)
# use a hardcoded API key to make sure one in the environment is never used
openai_client = openai.Client(api_key='foobar', http_client=httpx_client)

Expand Down Expand Up @@ -855,7 +855,7 @@ def test_dont_suppress_httpx(exporter: TestExporter) -> None:

def test_suppress_httpx(exporter: TestExporter) -> None:
with httpx.Client(transport=MockTransport(request_handler)) as httpx_client:
HTTPXClientInstrumentor.instrument_client(httpx_client)
HTTPXClientInstrumentor().instrument_client(httpx_client)
# use a hardcoded API key to make sure one in the environment is never used
openai_client = openai.Client(api_key='foobar', http_client=httpx_client)

Expand Down
3 changes: 3 additions & 0 deletions tests/test_configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,7 @@ def test_initialize_project_use_existing_project(tmp_dir_cwd: Path, tmp_path: Pa
'Project initialized successfully. You will be able to view it at: fake_project_url\nPress Enter to continue',
),
]
wait_for_check_token_thread()
assert capsys.readouterr().err == 'Logfire project URL: fake_project_url\n'

assert json.loads((tmp_dir_cwd / '.logfire/logfire_credentials.json').read_text()) == {
Expand Down Expand Up @@ -1027,6 +1028,7 @@ def test_initialize_project_not_using_existing_project(
'Project initialized successfully. You will be able to view it at: fake_project_url\nPress Enter to continue'
),
]
wait_for_check_token_thread()
assert capsys.readouterr().err == 'Logfire project URL: fake_project_url\n'

assert json.loads((tmp_dir_cwd / '.logfire/logfire_credentials.json').read_text()) == {
Expand Down Expand Up @@ -1296,6 +1298,7 @@ def test_send_to_logfire_if_token_present_empty_via_env_var() -> None:
side_effect=RuntimeError,
), requests_mock.Mocker() as requests_mocker:
configure(console=False)
wait_for_check_token_thread()
assert len(requests_mocker.request_history) == 0


Expand Down
34 changes: 33 additions & 1 deletion tests/test_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import pytest
import requests
from dirty_equals._numeric import IsInt
from dirty_equals import IsInt, IsOneOf
from inline_snapshot import snapshot
from opentelemetry.metrics import CallbackOptions, Observation
from opentelemetry.sdk.metrics._internal.export import MetricExporter, MetricExportResult
Expand Down Expand Up @@ -50,6 +50,7 @@ def test_create_metric_counter(metrics_reader: InMemoryMetricReader) -> None:
'start_time_unix_nano': IsInt(),
'time_unix_nano': IsInt(),
'value': 300 + 4000,
'exemplars': [],
}
],
'aggregation_temporality': AggregationTemporality.DELTA,
Expand Down Expand Up @@ -102,6 +103,7 @@ def test_create_metric_histogram(metrics_reader: InMemoryMetricReader) -> None:
'flags': 0,
'min': 300,
'max': 4000,
'exemplars': [],
}
],
'aggregation_temporality': AggregationTemporality.DELTA,
Expand All @@ -127,6 +129,7 @@ def test_create_metric_gauge(metrics_reader: InMemoryMetricReader) -> None:
'start_time_unix_nano': None,
'time_unix_nano': IsInt(),
'value': 1,
'exemplars': [],
}
]
},
Expand All @@ -150,6 +153,7 @@ def test_create_metric_gauge(metrics_reader: InMemoryMetricReader) -> None:
'start_time_unix_nano': None,
'time_unix_nano': IsInt(),
'value': 24,
'exemplars': [],
}
]
},
Expand Down Expand Up @@ -190,6 +194,7 @@ def test_create_metric_up_down_counter(metrics_reader: InMemoryMetricReader) ->
'start_time_unix_nano': IsInt(),
'time_unix_nano': IsInt(),
'value': 4321,
'exemplars': [],
}
],
'aggregation_temporality': AggregationTemporality.CUMULATIVE,
Expand Down Expand Up @@ -225,6 +230,15 @@ def observable_counter(options: CallbackOptions):
'start_time_unix_nano': IsInt(),
'time_unix_nano': IsInt(),
'value': 4300,
'exemplars': [
{
'filtered_attributes': None,
'value': 4321,
'time_unix_nano': IsInt(),
'span_id': None,
'trace_id': None,
}
],
}
],
'aggregation_temporality': AggregationTemporality.DELTA,
Expand Down Expand Up @@ -259,6 +273,15 @@ def observable_gauge(options: CallbackOptions):
'start_time_unix_nano': None,
'time_unix_nano': IsInt(),
'value': 4000,
'exemplars': [
{
'filtered_attributes': None,
'value': IsOneOf(300, 4000),
'time_unix_nano': IsInt(),
'span_id': None,
'trace_id': None,
}
],
}
]
},
Expand Down Expand Up @@ -292,6 +315,15 @@ def observable_counter(options: CallbackOptions):
'start_time_unix_nano': IsInt(),
'time_unix_nano': IsInt(),
'value': 4321,
'exemplars': [
{
'filtered_attributes': None,
'value': 4321,
'time_unix_nano': IsInt(),
'span_id': None,
'trace_id': None,
}
],
}
],
'aggregation_temporality': AggregationTemporality.CUMULATIVE,
Expand Down
Loading

0 comments on commit d8042bf

Please sign in to comment.