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

HTTP semantic convention stability migration for aiohttp-client #2673

Merged
merged 15 commits into from
Jul 15, 2024
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#2630](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2630))
- `opentelemetry-instrumentation-system-metrics` Add support for capture open file descriptors
([#2652](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2652))
- `opentelemetry-instrumentation-aiohttp-client` Implement new semantic convention opt-in migration
([#2673](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2673))

### Breaking changes

Expand Down
2 changes: 1 addition & 1 deletion instrumentation/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
| Instrumentation | Supported Packages | Metrics support | Semconv status |
| --------------- | ------------------ | --------------- | -------------- |
| [opentelemetry-instrumentation-aio-pika](./opentelemetry-instrumentation-aio-pika) | aio_pika >= 7.2.0, < 10.0.0 | No | experimental
| [opentelemetry-instrumentation-aiohttp-client](./opentelemetry-instrumentation-aiohttp-client) | aiohttp ~= 3.0 | No | experimental
| [opentelemetry-instrumentation-aiohttp-client](./opentelemetry-instrumentation-aiohttp-client) | aiohttp ~= 3.0 | No | migration
| [opentelemetry-instrumentation-aiohttp-server](./opentelemetry-instrumentation-aiohttp-server) | aiohttp ~= 3.0 | No | experimental
| [opentelemetry-instrumentation-aiopg](./opentelemetry-instrumentation-aiopg) | aiopg >= 0.13.0, < 2.0.0 | No | experimental
| [opentelemetry-instrumentation-asgi](./opentelemetry-instrumentation-asgi) | asgiref ~= 3.0 | Yes | migration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,19 +90,28 @@ def response_hook(span: Span, params: typing.Union[

from opentelemetry import context as context_api
from opentelemetry import trace
from opentelemetry.instrumentation._semconv import (
_get_schema_url,
_HTTPStabilityMode,
_OpenTelemetrySemanticConventionStability,
_OpenTelemetryStabilitySignalType,
_report_new,
_set_http_method,
_set_http_url,
_set_status,
)
from opentelemetry.instrumentation.aiohttp_client.package import _instruments
from opentelemetry.instrumentation.aiohttp_client.version import __version__
from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
from opentelemetry.instrumentation.utils import (
http_status_to_status_code,
is_instrumentation_enabled,
unwrap,
)
from opentelemetry.propagate import inject
from opentelemetry.semconv.trace import SpanAttributes
from opentelemetry.semconv.attributes.error_attributes import ERROR_TYPE
from opentelemetry.trace import Span, SpanKind, TracerProvider, get_tracer
from opentelemetry.trace.status import Status, StatusCode
from opentelemetry.util.http import remove_url_credentials
from opentelemetry.util.http import remove_url_credentials, sanitize_method

_UrlFilterT = typing.Optional[typing.Callable[[yarl.URL], str]]
_RequestHookT = typing.Optional[
Expand All @@ -122,11 +131,20 @@ def response_hook(span: Span, params: typing.Union[
]


def _get_span_name(method: str) -> str:
method = sanitize_method(method.upper().strip())
if method == "_OTHER":
method = "HTTP"

return method


def create_trace_config(
url_filter: _UrlFilterT = None,
request_hook: _RequestHookT = None,
response_hook: _ResponseHookT = None,
tracer_provider: TracerProvider = None,
sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT,
) -> aiohttp.TraceConfig:
"""Create an aiohttp-compatible trace configuration.

Expand Down Expand Up @@ -167,9 +185,12 @@ def create_trace_config(
__name__,
__version__,
tracer_provider,
schema_url="https://opentelemetry.io/schemas/1.11.0",
schema_url=_get_schema_url(sem_conv_opt_in_mode),
)

# TODO: Use this when we have durations for aiohttp-client
metrics_attributes = {}

def _end_trace(trace_config_ctx: types.SimpleNamespace):
context_api.detach(trace_config_ctx.token)
trace_config_ctx.span.end()
Expand All @@ -183,18 +204,22 @@ async def on_request_start(
trace_config_ctx.span = None
return

http_method = params.method.upper()
request_span_name = f"{http_method}"
http_method = params.method
request_span_name = _get_span_name(http_method)
request_url = (
remove_url_credentials(trace_config_ctx.url_filter(params.url))
if callable(trace_config_ctx.url_filter)
else remove_url_credentials(str(params.url))
)

span_attributes = {
SpanAttributes.HTTP_METHOD: http_method,
SpanAttributes.HTTP_URL: request_url,
}
span_attributes = {}
_set_http_method(
span_attributes,
http_method,
request_span_name,
sem_conv_opt_in_mode,
)
_set_http_url(span_attributes, request_url, sem_conv_opt_in_mode)

trace_config_ctx.span = trace_config_ctx.tracer.start_span(
request_span_name, kind=SpanKind.CLIENT, attributes=span_attributes
Expand All @@ -220,13 +245,24 @@ async def on_request_end(
if callable(response_hook):
response_hook(trace_config_ctx.span, params)

if trace_config_ctx.span.is_recording():
trace_config_ctx.span.set_status(
Status(http_status_to_status_code(int(params.response.status)))
)
trace_config_ctx.span.set_attribute(
SpanAttributes.HTTP_STATUS_CODE, params.response.status
)
status_code_str = str(params.response.status)
emdneto marked this conversation as resolved.
Show resolved Hide resolved

try:
status_code = int(params.response.status)
except ValueError:
status_code = -1
# When we have durations we should set metrics only once
# Also the decision to include status code on a histogram should
# not be dependent on tracing decisions.
server_span = False
_set_status(
trace_config_ctx.span,
metrics_attributes,
status_code,
status_code_str,
server_span,
sem_conv_opt_in_mode,
)
_end_trace(trace_config_ctx)

async def on_request_exception(
Expand All @@ -238,7 +274,13 @@ async def on_request_exception(
return

if trace_config_ctx.span.is_recording() and params.exception:
trace_config_ctx.span.set_status(Status(StatusCode.ERROR))
exc_type = type(params.exception).__qualname__
if _report_new(sem_conv_opt_in_mode):
trace_config_ctx.span.set_attribute(ERROR_TYPE, exc_type)

trace_config_ctx.span.set_status(
Status(StatusCode.ERROR, exc_type)
)
trace_config_ctx.span.record_exception(params.exception)

if callable(response_hook):
Expand Down Expand Up @@ -271,6 +313,7 @@ def _instrument(
trace_configs: typing.Optional[
typing.Sequence[aiohttp.TraceConfig]
] = None,
sem_conv_opt_in_mode: _HTTPStabilityMode = _HTTPStabilityMode.DEFAULT,
):
"""Enables tracing of all ClientSessions

Expand All @@ -293,6 +336,7 @@ def instrumented_init(wrapped, instance, args, kwargs):
request_hook=request_hook,
response_hook=response_hook,
tracer_provider=tracer_provider,
sem_conv_opt_in_mode=sem_conv_opt_in_mode,
)
trace_config._is_instrumented_by_opentelemetry = True
client_trace_configs.append(trace_config)
Expand Down Expand Up @@ -344,12 +388,17 @@ def _instrument(self, **kwargs):
``trace_configs``: An optional list of aiohttp.TraceConfig items, allowing customize enrichment of spans
based on aiohttp events (see specification: https://docs.aiohttp.org/en/stable/tracing_reference.html)
"""
_OpenTelemetrySemanticConventionStability._initialize()
_sem_conv_opt_in_mode = _OpenTelemetrySemanticConventionStability._get_opentelemetry_stability_opt_in_mode(
_OpenTelemetryStabilitySignalType.HTTP,
)
_instrument(
tracer_provider=kwargs.get("tracer_provider"),
url_filter=kwargs.get("url_filter"),
request_hook=kwargs.get("request_hook"),
response_hook=kwargs.get("response_hook"),
trace_configs=kwargs.get("trace_configs"),
sem_conv_opt_in_mode=_sem_conv_opt_in_mode,
)

def _uninstrument(self, **kwargs):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,7 @@


_instruments = ("aiohttp ~= 3.0",)

_supports_metrics = False

_semconv_status = "migration"
Loading