-
Notifications
You must be signed in to change notification settings - Fork 890
Restoring metrics in requests #1110
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
Changes from all commits
784bb02
feb34d8
0d58aaa
cc33431
c5224fc
f39393d
f497101
4cca547
7f843cb
b872ada
5c8cacb
68e34ee
80bdff2
7db639b
89d8f6e
b273409
52de6c3
661843b
8a0e994
077f4be
7f4a49b
32dab15
e57651b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,7 +50,9 @@ | |
|
|
||
| import functools | ||
| import types | ||
| from typing import Collection | ||
| from timeit import default_timer | ||
| from typing import Callable, Collection, Iterable, Optional | ||
| from urllib.parse import urlparse | ||
|
|
||
| from requests.models import Response | ||
| from requests.sessions import Session | ||
|
|
@@ -67,9 +69,11 @@ | |
| _SUPPRESS_INSTRUMENTATION_KEY, | ||
| http_status_to_status_code, | ||
| ) | ||
| from opentelemetry.metrics import Histogram, get_meter | ||
| from opentelemetry.propagate import inject | ||
| from opentelemetry.semconv.trace import SpanAttributes | ||
| from opentelemetry.trace import SpanKind, get_tracer | ||
| from opentelemetry.trace import SpanKind, Tracer, get_tracer | ||
| from opentelemetry.trace.span import Span | ||
| from opentelemetry.trace.status import Status | ||
| from opentelemetry.util.http import ( | ||
| get_excluded_urls, | ||
|
|
@@ -84,7 +88,11 @@ | |
| # pylint: disable=unused-argument | ||
| # pylint: disable=R0915 | ||
| def _instrument( | ||
| tracer, span_callback=None, name_callback=None, excluded_urls=None | ||
| tracer: Tracer, | ||
| duration_histogram: Histogram, | ||
| span_callback: Optional[Callable[[Span, Response], str]] = None, | ||
| name_callback: Optional[Callable[[str, str], str]] = None, | ||
| excluded_urls: Iterable[str] = None, | ||
| ): | ||
| """Enables tracing of all requests calls that go through | ||
| :code:`requests.session.Session.request` (this includes | ||
|
|
@@ -140,6 +148,7 @@ def call_wrapped(): | |
| request.method, request.url, call_wrapped, get_or_create_headers | ||
| ) | ||
|
|
||
| # pylint: disable-msg=too-many-locals,too-many-branches | ||
| def _instrumented_requests_call( | ||
| method: str, url: str, call_wrapped, get_or_create_headers | ||
| ): | ||
|
|
@@ -164,6 +173,23 @@ def _instrumented_requests_call( | |
| SpanAttributes.HTTP_URL: url, | ||
| } | ||
|
|
||
| metric_labels = { | ||
| SpanAttributes.HTTP_METHOD: method, | ||
| } | ||
|
|
||
| try: | ||
| parsed_url = urlparse(url) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I imagine that the call to
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In traces we add the url even if the url is invalid (e.g. url = |
||
| metric_labels[SpanAttributes.HTTP_SCHEME] = parsed_url.scheme | ||
| if parsed_url.hostname: | ||
| metric_labels[SpanAttributes.HTTP_HOST] = parsed_url.hostname | ||
| metric_labels[ | ||
| SpanAttributes.NET_PEER_NAME | ||
| ] = parsed_url.hostname | ||
| if parsed_url.port: | ||
| metric_labels[SpanAttributes.NET_PEER_PORT] = parsed_url.port | ||
| except ValueError: | ||
| pass | ||
|
|
||
| with tracer.start_as_current_span( | ||
| span_name, kind=SpanKind.CLIENT, attributes=span_attributes | ||
| ) as span, set_ip_on_next_http_connection(span): | ||
|
|
@@ -175,12 +201,18 @@ def _instrumented_requests_call( | |
| token = context.attach( | ||
| context.set_value(_SUPPRESS_HTTP_INSTRUMENTATION_KEY, True) | ||
| ) | ||
|
|
||
| start_time = default_timer() | ||
|
|
||
| try: | ||
| result = call_wrapped() # *** PROCEED | ||
| except Exception as exc: # pylint: disable=W0703 | ||
| exception = exc | ||
| result = getattr(exc, "response", None) | ||
| finally: | ||
| elapsed_time = max( | ||
| round((default_timer() - start_time) * 1000), 0 | ||
| ) | ||
| context.detach(token) | ||
|
|
||
| if isinstance(result, Response): | ||
|
|
@@ -191,9 +223,23 @@ def _instrumented_requests_call( | |
| span.set_status( | ||
| Status(http_status_to_status_code(result.status_code)) | ||
| ) | ||
|
|
||
| metric_labels[ | ||
| SpanAttributes.HTTP_STATUS_CODE | ||
| ] = result.status_code | ||
|
|
||
| if result.raw is not None: | ||
| version = getattr(result.raw, "version", None) | ||
| if version: | ||
| metric_labels[SpanAttributes.HTTP_FLAVOR] = ( | ||
| "1.1" if version == 11 else "1.0" | ||
| ) | ||
|
|
||
| if span_callback is not None: | ||
| span_callback(span, result) | ||
|
|
||
| duration_histogram.record(elapsed_time, attributes=metric_labels) | ||
|
|
||
| if exception is not None: | ||
| raise exception.with_traceback(exception.__traceback__) | ||
|
|
||
|
|
@@ -258,8 +304,20 @@ def _instrument(self, **kwargs): | |
| tracer_provider = kwargs.get("tracer_provider") | ||
| tracer = get_tracer(__name__, __version__, tracer_provider) | ||
| excluded_urls = kwargs.get("excluded_urls") | ||
| meter_provider = kwargs.get("meter_provider") | ||
| meter = get_meter( | ||
| __name__, | ||
| __version__, | ||
| meter_provider, | ||
| ) | ||
| duration_histogram = meter.create_histogram( | ||
| name="http.client.duration", | ||
| unit="ms", | ||
| description="measures the duration of the outbound HTTP request", | ||
| ) | ||
| _instrument( | ||
| tracer, | ||
| duration_histogram, | ||
| span_callback=kwargs.get("span_callback"), | ||
| name_callback=kwargs.get("name_callback"), | ||
| excluded_urls=_excluded_urls_from_env | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,3 +14,5 @@ | |
|
|
||
|
|
||
| _instruments = ("requests ~= 2.0",) | ||
|
|
||
| _supports_metrics = True | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -487,3 +487,47 @@ def perform_request(url: str, session: requests.Session = None): | |
| request = requests.Request("GET", url) | ||
| prepared_request = session.prepare_request(request) | ||
| return session.send(prepared_request) | ||
|
|
||
|
|
||
| class TestRequestsIntergrationMetric(TestBase): | ||
| URL = "http://examplehost:8000/status/200" | ||
|
|
||
| def setUp(self): | ||
| super().setUp() | ||
| RequestsInstrumentor().instrument(meter_provider=self.meter_provider) | ||
|
|
||
| httpretty.enable() | ||
| httpretty.register_uri(httpretty.GET, self.URL, body="Hello!") | ||
|
|
||
| def tearDown(self): | ||
| super().tearDown() | ||
| RequestsInstrumentor().uninstrument() | ||
| httpretty.disable() | ||
|
|
||
| @staticmethod | ||
| def perform_request(url: str) -> requests.Response: | ||
| return requests.get(url) | ||
|
|
||
| def test_basic_metric_success(self): | ||
| self.perform_request(self.URL) | ||
|
|
||
| expected_attributes = { | ||
| "http.status_code": 200, | ||
| "http.host": "examplehost", | ||
| "net.peer.port": 8000, | ||
| "net.peer.name": "examplehost", | ||
| "http.method": "GET", | ||
| "http.flavor": "1.1", | ||
| "http.scheme": "http", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we missing a few?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||
| } | ||
|
|
||
| for ( | ||
| resource_metrics | ||
| ) in self.memory_metrics_reader.get_metrics_data().resource_metrics: | ||
| for scope_metrics in resource_metrics.scope_metrics: | ||
| for metric in scope_metrics.metrics: | ||
| for data_point in metric.data.data_points: | ||
| self.assertDictEqual( | ||
| expected_attributes, dict(data_point.attributes) | ||
| ) | ||
| self.assertEqual(data_point.count, 1) | ||
Uh oh!
There was an error while loading. Please reload this page.