Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow instrumenting a single httpx client, fix tests for OTel 1.28 #575

Merged
merged 8 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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!"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pyright was failing to parse this file because of the emojis: microsoft/pyright#9412

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is sad.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is closed, upgrading pyright should allow reverting this.

requires-python = ">=3.8"
authors = [
{ name = "Pydantic Team", email = "[email protected]" },
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",
alexmojaki marked this conversation as resolved.
Show resolved Hide resolved
]
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