Skip to content

Commit

Permalink
Merge branch 'main' into feature/http-route-in-metric
Browse files Browse the repository at this point in the history
  • Loading branch information
GonzaloGuasch authored Jul 13, 2024
2 parents 335657e + 6bc48be commit 077b410
Show file tree
Hide file tree
Showing 12 changed files with 700 additions and 84 deletions.
9 changes: 7 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#2638](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2638))
- `opentelemetry-instrumentation-asgi` Implement new semantic convention opt-in with stable http semantic conventions
([#2610](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2610))
- `opentelemetry-instrumentation-fastapi` Implement new semantic convention opt-in with stable http semantic conventions
([#2682](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2682))
- `opentelemetry-instrumentation-httpx` Implement new semantic convention opt-in migration with stable http semantic conventions
([#2631](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2631))
- `opentelemetry-instrumentation-system-metrics` Permit to use psutil 6.0+.
Expand All @@ -34,9 +36,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- `opentelemetry-instrumentation-asgi`, `opentelemetry-instrumentation-fastapi`, `opentelemetry-instrumentation-starlette` Use `tracer` and `meter` of originating components instead of one from `asgi` middleware
([#2580](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2580))
- Populate `{method}` as `HTTP` on `_OTHER` methods from scope
- Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `asgi` middleware
([#2610](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2610))

- Populate `{method}` as `HTTP` on `_OTHER` methods from scope for `fastapi` middleware
([#2682](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2682))

### Fixed

Expand All @@ -63,6 +66,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#2153](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2153))
- `opentelemetry-instrumentation-asgi` Removed `NET_HOST_NAME` AND `NET_HOST_PORT` from active requests count attribute
([#2610](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2610))
- `opentelemetry-instrumentation-asgi` Bugfix: Middleware did not set status code attribute on duration metrics for non-recording spans.
([#2627](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2627))


## Version 1.25.0/0.46b0 (2024-05-31)
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ Meeting notes are available as a public [Google doc](https://docs.google.com/doc
Approvers ([@open-telemetry/python-approvers](https://github.com/orgs/open-telemetry/teams/python-approvers)):

- [Aaron Abbott](https://github.com/aabmass), Google
- [Emídio Neto](https://github.com/emdneto), Zenvia
- [Jeremy Voss](https://github.com/jeremydvoss), Microsoft
- [Owais Lone](https://github.com/owais), Splunk
- [Pablo Collins](https://github.com/pmcollins), Splunk
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
asgiref==3.7.2
certifi==2024.2.2
certifi==2024.7.4
charset-normalizer==3.3.2
# We can drop this after bumping baseline to pypy-39
cramjam==2.1.0; platform_python_implementation == "PyPy"
Expand Down
2 changes: 1 addition & 1 deletion instrumentation/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
| [opentelemetry-instrumentation-django](./opentelemetry-instrumentation-django) | django >= 1.10 | Yes | experimental
| [opentelemetry-instrumentation-elasticsearch](./opentelemetry-instrumentation-elasticsearch) | elasticsearch >= 6.0 | No | experimental
| [opentelemetry-instrumentation-falcon](./opentelemetry-instrumentation-falcon) | falcon >= 1.4.1, < 4.0.0 | Yes | experimental
| [opentelemetry-instrumentation-fastapi](./opentelemetry-instrumentation-fastapi) | fastapi ~= 0.58 | Yes | experimental
| [opentelemetry-instrumentation-fastapi](./opentelemetry-instrumentation-fastapi) | fastapi ~= 0.58 | Yes | migration
| [opentelemetry-instrumentation-flask](./opentelemetry-instrumentation-flask) | flask >= 1.0 | Yes | migration
| [opentelemetry-instrumentation-grpc](./opentelemetry-instrumentation-grpc) | grpcio ~= 1.27 | No | experimental
| [opentelemetry-instrumentation-httpx](./opentelemetry-instrumentation-httpx) | httpx >= 0.18.0 | No | migration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -438,8 +438,6 @@ def set_status_code(
sem_conv_opt_in_mode=_HTTPStabilityMode.DEFAULT,
):
"""Adds HTTP response attributes to span using the status_code argument."""
if not span.is_recording():
return
status_code_str = str(status_code)

try:
Expand Down Expand Up @@ -836,36 +834,16 @@ async def otel_send(message: dict[str, Any]):
) 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":
status_code = message["status"]
# 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,
)
expecting_trailers = message.get("trailers", False)
elif message["type"] == "websocket.send":
set_status_code(
server_span,
200,
duration_attrs,
self._sem_conv_opt_in_mode,
)
set_status_code(
send_span,
200,
None,
self._sem_conv_opt_in_mode,
)
send_span.set_attribute("asgi.event.type", message["type"])
if (
server_span.is_recording()
Expand All @@ -886,6 +864,20 @@ async def otel_send(message: dict[str, Any]):
server_span.set_attributes(
custom_response_attributes
)
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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,9 @@ def test_asgi_not_recording(self):
mock_span = mock.Mock()
mock_span.is_recording.return_value = False
mock_tracer.start_as_current_span.return_value = mock_span
mock_tracer.start_as_current_span.return_value.__enter__ = mock_span
mock_tracer.start_as_current_span.return_value.__enter__ = mock.Mock(
return_value=mock_span
)
mock_tracer.start_as_current_span.return_value.__exit__ = mock_span
with mock.patch("opentelemetry.trace.get_tracer") as tracer:
tracer.return_value = mock_tracer
Expand Down Expand Up @@ -1342,6 +1344,65 @@ def test_basic_metric_success(self):
)
self.assertEqual(point.value, 0)

def test_basic_metric_success_nonrecording_span(self):
mock_tracer = mock.Mock()
mock_span = mock.Mock()
mock_span.is_recording.return_value = False
mock_tracer.start_as_current_span.return_value = mock_span
mock_tracer.start_as_current_span.return_value.__enter__ = mock.Mock(
return_value=mock_span
)
mock_tracer.start_as_current_span.return_value.__exit__ = mock_span
with mock.patch("opentelemetry.trace.get_tracer") as tracer:
tracer.return_value = mock_tracer
app = otel_asgi.OpenTelemetryMiddleware(simple_asgi)
self.seed_app(app)
start = default_timer()
self.send_default_request()
duration = max(round((default_timer() - start) * 1000), 0)
expected_duration_attributes = {
"http.method": "GET",
"http.host": "127.0.0.1",
"http.scheme": "http",
"http.flavor": "1.0",
"net.host.port": 80,
"http.status_code": 200,
}
expected_requests_count_attributes = {
"http.method": "GET",
"http.host": "127.0.0.1",
"http.scheme": "http",
"http.flavor": "1.0",
}
metrics_list = self.memory_metrics_reader.get_metrics_data()
# pylint: disable=too-many-nested-blocks
for resource_metric in metrics_list.resource_metrics:
for scope_metrics in resource_metric.scope_metrics:
for metric in scope_metrics.metrics:
for point in list(metric.data.data_points):
if isinstance(point, HistogramDataPoint):
self.assertDictEqual(
expected_duration_attributes,
dict(point.attributes),
)
self.assertEqual(point.count, 1)
if metric.name == "http.server.duration":
self.assertAlmostEqual(
duration, point.sum, delta=5
)
elif (
metric.name == "http.server.response.size"
):
self.assertEqual(1024, point.sum)
elif metric.name == "http.server.request.size":
self.assertEqual(128, point.sum)
elif isinstance(point, NumberDataPoint):
self.assertDictEqual(
expected_requests_count_attributes,
dict(point.attributes),
)
self.assertEqual(point.value, 0)

def test_basic_metric_success_new_semconv(self):
app = otel_asgi.OpenTelemetryMiddleware(simple_asgi)
self.seed_app(app)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,12 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A
import fastapi
from starlette.routing import Match

from opentelemetry.instrumentation._semconv import (
_get_schema_url,
_HTTPStabilityMode,
_OpenTelemetrySemanticConventionStability,
_OpenTelemetryStabilitySignalType,
)
from opentelemetry.instrumentation.asgi import OpenTelemetryMiddleware
from opentelemetry.instrumentation.asgi.types import (
ClientRequestHook,
Expand All @@ -189,7 +195,11 @@ def client_response_hook(span: Span, scope: dict[str, Any], message: dict[str, A
from opentelemetry.metrics import get_meter
from opentelemetry.semconv.trace import SpanAttributes
from opentelemetry.trace import get_tracer
from opentelemetry.util.http import get_excluded_urls, parse_excluded_urls
from opentelemetry.util.http import (
get_excluded_urls,
parse_excluded_urls,
sanitize_method,
)

_excluded_urls_from_env = get_excluded_urls("FASTAPI")
_logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -218,6 +228,11 @@ def instrument_app(
app._is_instrumented_by_opentelemetry = False

if not getattr(app, "_is_instrumented_by_opentelemetry", False):
# initialize semantic conventions opt-in if needed
_OpenTelemetrySemanticConventionStability._initialize()
sem_conv_opt_in_mode = _OpenTelemetrySemanticConventionStability._get_opentelemetry_stability_opt_in_mode(
_OpenTelemetryStabilitySignalType.HTTP,
)
if excluded_urls is None:
excluded_urls = _excluded_urls_from_env
else:
Expand All @@ -226,13 +241,13 @@ def instrument_app(
__name__,
__version__,
tracer_provider,
schema_url="https://opentelemetry.io/schemas/1.11.0",
schema_url=_get_schema_url(sem_conv_opt_in_mode),
)
meter = get_meter(
__name__,
__version__,
meter_provider,
schema_url="https://opentelemetry.io/schemas/1.11.0",
schema_url=_get_schema_url(sem_conv_opt_in_mode),
)

app.add_middleware(
Expand Down Expand Up @@ -303,20 +318,25 @@ class _InstrumentedFastAPI(fastapi.FastAPI):
_client_request_hook: ClientRequestHook = None
_client_response_hook: ClientResponseHook = None
_instrumented_fastapi_apps = set()
_sem_conv_opt_in_mode = _HTTPStabilityMode.DEFAULT

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
tracer = get_tracer(
__name__,
__version__,
_InstrumentedFastAPI._tracer_provider,
schema_url="https://opentelemetry.io/schemas/1.11.0",
schema_url=_get_schema_url(
_InstrumentedFastAPI._sem_conv_opt_in_mode
),
)
meter = get_meter(
__name__,
__version__,
_InstrumentedFastAPI._meter_provider,
schema_url="https://opentelemetry.io/schemas/1.11.0",
schema_url=_get_schema_url(
_InstrumentedFastAPI._sem_conv_opt_in_mode
),
)
self.add_middleware(
OpenTelemetryMiddleware,
Expand Down Expand Up @@ -373,8 +393,10 @@ def _get_default_span_details(scope):
A tuple of span name and attributes
"""
route = _get_route_details(scope)
method = scope.get("method", "")
method = sanitize_method(scope.get("method", "").strip())
attributes = {}
if method == "_OTHER":
method = "HTTP"
if route:
attributes[SpanAttributes.HTTP_ROUTE] = route
if method and route: # http
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,5 @@
_instruments = ("fastapi ~= 0.58",)

_supports_metrics = True

_semconv_status = "migration"
Loading

0 comments on commit 077b410

Please sign in to comment.