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

feat: add ability to optionally disable internal HTTP send and receive spans #2802

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
85648ee
feat: add option to disable internal send and receive spans
omgitsaheadcrab Aug 16, 2024
5b7759a
fix: broken custom headers
omgitsaheadcrab Aug 16, 2024
b6faf30
test(TestAsgiApplication): add test for internal span exclusion
omgitsaheadcrab Aug 16, 2024
15b4d60
chore: update CHANGELOG
omgitsaheadcrab Aug 16, 2024
90943e9
Merge branch 'main' into optional-send-receive-spans
omgitsaheadcrab Aug 16, 2024
2073d2a
refactor: remove environment variables for internal span toggle
omgitsaheadcrab Aug 19, 2024
906721b
chore: remove unused import
omgitsaheadcrab Aug 19, 2024
0741bc6
refactor(FastAPIInstrumentor): add ability to exclude internal send a…
omgitsaheadcrab Aug 19, 2024
839dbf9
refactor: remove spurious addition
omgitsaheadcrab Aug 19, 2024
5b6ea8e
Merge branch 'optional-send-receive-spans-fastapi' into optional-send…
omgitsaheadcrab Aug 19, 2024
52bd660
docs: update CHANGELOG
omgitsaheadcrab Aug 19, 2024
b1e445e
fix: add missing instrumentation
omgitsaheadcrab Aug 19, 2024
1cc0495
Merge branch 'main' into optional-send-receive-spans
lzchen Aug 19, 2024
a5bfd43
chore: fix too-many-branches ignore
omgitsaheadcrab Aug 20, 2024
9daafa7
docs: appease sphinx autodoc
omgitsaheadcrab Aug 20, 2024
9c32d80
fix: server span logic should always be executed if recording
omgitsaheadcrab Aug 21, 2024
f312d4a
refactor: combine exclusions into one optional list
omgitsaheadcrab Aug 21, 2024
c074c62
docs: update wording
omgitsaheadcrab Aug 21, 2024
ddfb5e6
style: format code
omgitsaheadcrab Aug 21, 2024
3fc263e
refactor: extract set_send_span and set_server_span logic to helpers
omgitsaheadcrab Aug 21, 2024
3f36b70
Merge branch 'main' into optional-send-receive-spans
lzchen Aug 22, 2024
09a2ef4
test: add assertion to ensure list always has at least the server spans
omgitsaheadcrab Aug 23, 2024
be1ebbf
refactor: apply suggestions from code review
omgitsaheadcrab Aug 23, 2024
f7b2a51
Merge branch 'main' into optional-send-receive-spans
lzchen Aug 23, 2024
6b2490a
Merge branch 'main' into optional-send-receive-spans
lzchen Aug 23, 2024
59ef359
Merge branch 'main' into optional-send-receive-spans
lzchen Aug 26, 2024
7ec823d
Merge branch 'main' into optional-send-receive-spans
omgitsaheadcrab Sep 5, 2024
6909756
test: ensure memory exporter cleared between cases
omgitsaheadcrab Sep 9, 2024
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `opentelemetry-instrumentation-kafka-python` Instrument temporary fork, kafka-python-ng
inside kafka-python's instrumentation
([#2537](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2537))
- `opentelemetry-instrumentation-asgi` Add ability to disable internal HTTP send and receive spans
([#2802](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2802))

## Breaking changes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,15 +185,22 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A
Note:
The environment variable names used to capture HTTP headers are still experimental, and thus are subject to change.

Internal HTTP spans
*****************************
Internal HTTP send and receive spans are added by default. These can optionally be excluded by setting the boolean environment variables
``OTEL_PYTHON_ASGI_EXCLUDE_SEND_SPAN`` and ``OTEL_PYTHON_ASGI_EXCLUDE_RECEIVE_SPAN`` to ``true``.

API
---
"""

from __future__ import annotations

import os
import typing
import urllib
from collections import defaultdict
from distutils.util import strtobool
from functools import wraps
from timeit import default_timer
from typing import Any, Awaitable, Callable, DefaultDict, Tuple
Expand Down Expand Up @@ -226,6 +233,10 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A
_set_http_user_agent,
_set_status,
)
from opentelemetry.instrumentation.asgi.environment_variables import (
OTEL_PYTHON_ASGI_EXCLUDE_RECEIVE_SPAN,
OTEL_PYTHON_ASGI_EXCLUDE_SEND_SPAN,
)
from opentelemetry.instrumentation.asgi.types import (
ClientRequestHook,
ClientResponseHook,
Expand Down Expand Up @@ -481,7 +492,7 @@ def get_default_span_details(scope: dict) -> Tuple[str, dict]:


def _collect_target_attribute(
scope: typing.Dict[str, typing.Any]
scope: typing.Dict[str, typing.Any],
) -> typing.Optional[str]:
"""
Returns the target path as defined by the Semantic Conventions.
Expand Down Expand Up @@ -527,6 +538,8 @@ class OpenTelemetryMiddleware:
the current globally configured one is used.
meter_provider: The optional meter provider to use. If omitted
the current globally configured one is used.
exclude_receive_span: Optional flag to exclude the http receive span from the trace.
exclude_send_span: Optional flag to exclude the http send span from the trace.
"""

# pylint: disable=too-many-branches
Expand All @@ -545,6 +558,8 @@ def __init__(
http_capture_headers_server_request: list[str] | None = None,
http_capture_headers_server_response: list[str] | None = None,
http_capture_headers_sanitize_fields: list[str] | None = None,
exclude_receive_span: bool = False,
exclude_send_span: bool = False,
xrmx marked this conversation as resolved.
Show resolved Hide resolved
):
# initialize semantic conventions opt-in if needed
_OpenTelemetrySemanticConventionStability._initialize()
Expand Down Expand Up @@ -651,6 +666,12 @@ def __init__(
)
or []
)
self.exclude_receive_span = exclude_receive_span or strtobool(
os.getenv(OTEL_PYTHON_ASGI_EXCLUDE_RECEIVE_SPAN, "false")
lzchen marked this conversation as resolved.
Show resolved Hide resolved
)
self.exclude_send_span = exclude_send_span or strtobool(
os.getenv(OTEL_PYTHON_ASGI_EXCLUDE_SEND_SPAN, "false")
)

# pylint: disable=too-many-statements
async def __call__(
Expand Down Expand Up @@ -796,6 +817,9 @@ async def __call__(
# pylint: enable=too-many-branches

def _get_otel_receive(self, server_span_name, scope, receive):
if self.exclude_receive_span:
return receive

@wraps(receive)
async def otel_receive():
with self.tracer.start_as_current_span(
Expand Down Expand Up @@ -832,74 +856,82 @@ def _get_otel_send(
@wraps(send)
async def otel_send(message: dict[str, Any]):
nonlocal expecting_trailers
with self.tracer.start_as_current_span(
" ".join((server_span_name, scope["type"], "send"))
) as send_span:
if callable(self.client_response_hook):
self.client_response_hook(send_span, scope, message)

status_code = None
if message["type"] == "http.response.start":
status_code = message["status"]
elif message["type"] == "websocket.send":
status_code = 200

if send_span.is_recording():
if message["type"] == "http.response.start":
expecting_trailers = message.get("trailers", False)
send_span.set_attribute("asgi.event.type", message["type"])
if (
server_span.is_recording()
and server_span.kind == trace.SpanKind.SERVER
and "headers" in message
):
custom_response_attributes = (
collect_custom_headers_attributes(
message,
self.http_capture_headers_sanitize_fields,
self.http_capture_headers_server_response,
normalise_response_header_name,
)
if self.http_capture_headers_server_response
else {}

status_code = None
if message["type"] == "http.response.start":
status_code = message["status"]
expecting_trailers = message.get("trailers", False)
xrmx marked this conversation as resolved.
Show resolved Hide resolved
elif message["type"] == "websocket.send":
status_code = 200

if not self.exclude_send_span:
with self.tracer.start_as_current_span(
" ".join((server_span_name, scope["type"], "send"))
) as send_span:
if callable(self.client_response_hook):
self.client_response_hook(send_span, scope, message)

if send_span.is_recording():
if message["type"] == "http.response.start":
expecting_trailers = message.get("trailers", False)
send_span.set_attribute(
"asgi.event.type", message["type"]
)
if len(custom_response_attributes) > 0:
server_span.set_attributes(
custom_response_attributes
if status_code:
set_status_code(
send_span,
status_code,
None,
self._sem_conv_opt_in_mode,
)
if status_code:
# We record metrics only once
set_status_code(
server_span,
status_code,
duration_attrs,
self._sem_conv_opt_in_mode,
)
set_status_code(
send_span,
status_code,
None,
self._sem_conv_opt_in_mode,
)

propagator = get_global_response_propagator()
if propagator:
propagator.inject(
message,
context=set_span_in_context(
server_span, trace.context_api.Context()
),
setter=asgi_setter,
)
if (
lzchen marked this conversation as resolved.
Show resolved Hide resolved
server_span.is_recording()
and server_span.kind == trace.SpanKind.SERVER
and "headers" in message
):
custom_response_attributes = (
collect_custom_headers_attributes(
message,
self.http_capture_headers_sanitize_fields,
self.http_capture_headers_server_response,
normalise_response_header_name,
)
if self.http_capture_headers_server_response
else {}
)
if len(custom_response_attributes) > 0:
server_span.set_attributes(
custom_response_attributes
)

if status_code:
set_status_code(
server_span,
status_code,
duration_attrs,
self._sem_conv_opt_in_mode,
)

content_length = asgi_getter.get(message, "content-length")
if content_length:
try:
self.content_length_header = int(content_length[0])
except ValueError:
pass
propagator = get_global_response_propagator()
if propagator:
propagator.inject(
message,
context=set_span_in_context(
server_span, trace.context_api.Context()
),
setter=asgi_setter,
)

content_length = asgi_getter.get(message, "content-length")
if content_length:
try:
self.content_length_header = int(content_length[0])
except ValueError:
pass

await send(message)

await send(message)
# pylint: disable=too-many-boolean-expressions
if (
not expecting_trailers
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Copyright The OpenTelemetry Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""
Exclude the http send span from the tracing feature in Python.
"""

OTEL_PYTHON_ASGI_EXCLUDE_SEND_SPAN = "OTEL_PYTHON_ASGI_EXCLUDE_SEND_SPAN"
lzchen marked this conversation as resolved.
Show resolved Hide resolved

"""
Exclude the http receive span from the tracing feature in Python.
"""
OTEL_PYTHON_ASGI_EXCLUDE_RECEIVE_SPAN = "OTEL_PYTHON_ASGI_EXCLUDE_RECEIVE_SPAN"
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,27 @@ def test_background_execution(self):
_SIMULATED_BACKGROUND_TASK_EXECUTION_TIME_S * 10**9,
)

def test_exclude_internal_spans(self):
"""Test that internal spans are excluded from the emitted spans when
the `exclude_receive_span` or `exclude_send_span` attributes are set.
"""
app = otel_asgi.OpenTelemetryMiddleware(simple_asgi)
self.seed_app(app)
cases = [
(True, True, ["GET / http receive", "GET / http send"]),
(False, True, ["GET / http send"]),
(True, False, ["GET / http receive"]),
(False, False, []),
]
for exclude_receive_span, exclude_send_span, excluded_spans in cases:
app.exclude_receive_span = exclude_receive_span
app.exclude_send_span = exclude_send_span
self.send_default_request()
span_list = self.memory_exporter.get_finished_spans()
xrmx marked this conversation as resolved.
Show resolved Hide resolved
for span in span_list:
for excluded_span in excluded_spans:
self.assertNotEqual(span.name, excluded_span)

def test_trailers(self):
"""Test that trailers are emitted as expected and that the server span is ended
BEFORE the background task is finished."""
Expand Down