Skip to content

Commit 766fd33

Browse files
xrmxxrmx
committed
Reduce use of shared state in the instrumentor class
1 parent e6d20b7 commit 766fd33

File tree

1 file changed

+103
-58
lines changed
  • instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx

1 file changed

+103
-58
lines changed

‎instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py‎

Lines changed: 103 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ async def async_response_hook(span, request, response):
193193
import logging
194194
import typing
195195
from asyncio import iscoroutinefunction
196+
from functools import partial
196197
from types import TracebackType
197198

198199
import httpx
@@ -731,45 +732,53 @@ def _instrument(self, **kwargs):
731732
``async_response_hook``: Async``response_hook`` for ``httpx.AsyncClient``
732733
"""
733734
tracer_provider = kwargs.get("tracer_provider")
734-
_request_hook = kwargs.get("request_hook")
735-
self._request_hook = _request_hook if callable(_request_hook) else None
736-
_response_hook = kwargs.get("response_hook")
737-
self._response_hook = (
738-
_response_hook if callable(_response_hook) else None
739-
)
740-
_async_request_hook = kwargs.get("async_request_hook")
741-
self._async_request_hook = (
742-
_async_request_hook
743-
if iscoroutinefunction(_async_request_hook)
735+
request_hook = kwargs.get("request_hook")
736+
response_hook = kwargs.get("response_hook")
737+
async_request_hook = kwargs.get("async_request_hook")
738+
async_request_hook = (
739+
async_request_hook
740+
if iscoroutinefunction(async_request_hook)
744741
else None
745742
)
746-
_async_response_hook = kwargs.get("async_response_hook")
747-
self._async_response_hook = (
748-
_async_response_hook
749-
if iscoroutinefunction(_async_response_hook)
743+
async_response_hook = kwargs.get("async_response_hook")
744+
async_response_hook = (
745+
async_response_hook
746+
if iscoroutinefunction(async_response_hook)
750747
else None
751748
)
752749

753750
_OpenTelemetrySemanticConventionStability._initialize()
754-
self._sem_conv_opt_in_mode = _OpenTelemetrySemanticConventionStability._get_opentelemetry_stability_opt_in_mode(
751+
sem_conv_opt_in_mode = _OpenTelemetrySemanticConventionStability._get_opentelemetry_stability_opt_in_mode(
755752
_OpenTelemetryStabilitySignalType.HTTP,
756753
)
757-
self._tracer = get_tracer(
754+
tracer = get_tracer(
758755
__name__,
759756
instrumenting_library_version=__version__,
760757
tracer_provider=tracer_provider,
761-
schema_url=_get_schema_url(self._sem_conv_opt_in_mode),
758+
schema_url=_get_schema_url(sem_conv_opt_in_mode),
762759
)
763760

764761
wrap_function_wrapper(
765762
"httpx",
766763
"HTTPTransport.handle_request",
767-
self._handle_request_wrapper,
764+
partial(
765+
self._handle_request_wrapper,
766+
tracer=tracer,
767+
sem_conv_opt_in_mode=sem_conv_opt_in_mode,
768+
request_hook=request_hook,
769+
response_hook=response_hook,
770+
),
768771
)
769772
wrap_function_wrapper(
770773
"httpx",
771774
"AsyncHTTPTransport.handle_async_request",
772-
self._handle_async_request_wrapper,
775+
partial(
776+
self._handle_async_request_wrapper,
777+
tracer=tracer,
778+
sem_conv_opt_in_mode=sem_conv_opt_in_mode,
779+
async_request_hook=async_request_hook,
780+
async_response_hook=async_response_hook,
781+
),
773782
)
774783

775784
def _uninstrument(self, **kwargs):
@@ -778,7 +787,17 @@ def _uninstrument(self, **kwargs):
778787
unwrap(httpx.HTTPTransport, "handle_request")
779788
unwrap(httpx.AsyncHTTPTransport, "handle_async_request")
780789

781-
def _handle_request_wrapper(self, wrapped, instance, args, kwargs):
790+
def _handle_request_wrapper(
791+
self,
792+
wrapped,
793+
instance,
794+
args,
795+
kwargs,
796+
tracer,
797+
sem_conv_opt_in_mode,
798+
request_hook,
799+
response_hook,
800+
):
782801
if not is_http_instrumentation_enabled():
783802
return wrapped(*args, **kwargs)
784803

@@ -793,17 +812,17 @@ def _handle_request_wrapper(self, wrapped, instance, args, kwargs):
793812
span_attributes,
794813
url,
795814
method_original,
796-
self._sem_conv_opt_in_mode,
815+
sem_conv_opt_in_mode,
797816
)
798817

799818
request_info = RequestInfo(method, url, headers, stream, extensions)
800819

801-
with self._tracer.start_as_current_span(
820+
with tracer.start_as_current_span(
802821
span_name, kind=SpanKind.CLIENT, attributes=span_attributes
803822
) as span:
804823
exception = None
805-
if callable(self._request_hook):
806-
self._request_hook(span, request_info)
824+
if callable(request_hook):
825+
request_hook(span, request_info)
807826

808827
_inject_propagation_headers(headers, args, kwargs)
809828

@@ -824,19 +843,17 @@ def _handle_request_wrapper(self, wrapped, instance, args, kwargs):
824843
span,
825844
status_code,
826845
http_version,
827-
self._sem_conv_opt_in_mode,
846+
sem_conv_opt_in_mode,
828847
)
829-
if callable(self._response_hook):
830-
self._response_hook(
848+
if callable(response_hook):
849+
response_hook(
831850
span,
832851
request_info,
833852
ResponseInfo(status_code, headers, stream, extensions),
834853
)
835854

836855
if exception:
837-
if span.is_recording() and _report_new(
838-
self._sem_conv_opt_in_mode
839-
):
856+
if span.is_recording() and _report_new(sem_conv_opt_in_mode):
840857
span.set_attribute(
841858
ERROR_TYPE, type(exception).__qualname__
842859
)
@@ -845,7 +862,15 @@ def _handle_request_wrapper(self, wrapped, instance, args, kwargs):
845862
return response
846863

847864
async def _handle_async_request_wrapper(
848-
self, wrapped, instance, args, kwargs
865+
self,
866+
wrapped,
867+
instance,
868+
args,
869+
kwargs,
870+
tracer,
871+
sem_conv_opt_in_mode,
872+
async_request_hook,
873+
async_response_hook,
849874
):
850875
if not is_http_instrumentation_enabled():
851876
return await wrapped(*args, **kwargs)
@@ -861,17 +886,17 @@ async def _handle_async_request_wrapper(
861886
span_attributes,
862887
url,
863888
method_original,
864-
self._sem_conv_opt_in_mode,
889+
sem_conv_opt_in_mode,
865890
)
866891

867892
request_info = RequestInfo(method, url, headers, stream, extensions)
868893

869-
with self._tracer.start_as_current_span(
894+
with tracer.start_as_current_span(
870895
span_name, kind=SpanKind.CLIENT, attributes=span_attributes
871896
) as span:
872897
exception = None
873-
if callable(self._async_request_hook):
874-
await self._async_request_hook(span, request_info)
898+
if callable(async_request_hook):
899+
await async_request_hook(span, request_info)
875900

876901
_inject_propagation_headers(headers, args, kwargs)
877902

@@ -892,20 +917,18 @@ async def _handle_async_request_wrapper(
892917
span,
893918
status_code,
894919
http_version,
895-
self._sem_conv_opt_in_mode,
920+
sem_conv_opt_in_mode,
896921
)
897922

898-
if callable(self._async_response_hook):
899-
await self._async_response_hook(
923+
if callable(async_response_hook):
924+
await async_response_hook(
900925
span,
901926
request_info,
902927
ResponseInfo(status_code, headers, stream, extensions),
903928
)
904929

905930
if exception:
906-
if span.is_recording() and _report_new(
907-
self._sem_conv_opt_in_mode
908-
):
931+
if span.is_recording() and _report_new(sem_conv_opt_in_mode):
909932
span.set_attribute(
910933
ERROR_TYPE, type(exception).__qualname__
911934
)
@@ -941,59 +964,81 @@ def instrument_client(
941964
)
942965
return
943966

944-
# FIXME: sharing state in the instrumentor instance maybe it's not that great, need to pass tracer and semconv to each
945-
# instance separately
946967
_OpenTelemetrySemanticConventionStability._initialize()
947-
self._sem_conv_opt_in_mode = _OpenTelemetrySemanticConventionStability._get_opentelemetry_stability_opt_in_mode(
968+
sem_conv_opt_in_mode = _OpenTelemetrySemanticConventionStability._get_opentelemetry_stability_opt_in_mode(
948969
_OpenTelemetryStabilitySignalType.HTTP,
949970
)
950-
self._tracer = get_tracer(
971+
tracer = get_tracer(
951972
__name__,
952973
instrumenting_library_version=__version__,
953974
tracer_provider=tracer_provider,
954-
schema_url=_get_schema_url(self._sem_conv_opt_in_mode),
975+
schema_url=_get_schema_url(sem_conv_opt_in_mode),
955976
)
956977

957978
if iscoroutinefunction(request_hook):
958-
self._async_request_hook = request_hook
959-
self._request_hook = None
979+
async_request_hook = request_hook
980+
request_hook = None
960981
else:
961-
self._request_hook = request_hook
962-
self._async_request_hook = None
982+
request_hook = request_hook
983+
async_request_hook = None
963984

964985
if iscoroutinefunction(response_hook):
965-
self._async_response_hook = response_hook
966-
self._response_hook = None
986+
async_response_hook = response_hook
987+
response_hook = None
967988
else:
968-
self._response_hook = response_hook
969-
self._async_response_hook = None
989+
response_hook = response_hook
990+
async_response_hook = None
970991

971992
if hasattr(client._transport, "handle_request"):
972993
wrap_function_wrapper(
973994
client._transport,
974995
"handle_request",
975-
self._handle_request_wrapper,
996+
partial(
997+
self._handle_request_wrapper,
998+
tracer=tracer,
999+
sem_conv_opt_in_mode=sem_conv_opt_in_mode,
1000+
request_hook=request_hook,
1001+
response_hook=response_hook,
1002+
),
9761003
)
9771004
for transport in client._mounts.values():
9781005
# FIXME: check it's not wrapped already?
9791006
wrap_function_wrapper(
9801007
transport,
9811008
"handle_request",
982-
self._handle_request_wrapper,
1009+
partial(
1010+
self._handle_request_wrapper,
1011+
tracer=tracer,
1012+
sem_conv_opt_in_mode=sem_conv_opt_in_mode,
1013+
request_hook=request_hook,
1014+
response_hook=response_hook,
1015+
),
9831016
)
9841017
client._is_instrumented_by_opentelemetry = True
9851018
if hasattr(client._transport, "handle_async_request"):
9861019
wrap_function_wrapper(
9871020
client._transport,
9881021
"handle_async_request",
989-
self._handle_async_request_wrapper,
1022+
partial(
1023+
self._handle_async_request_wrapper,
1024+
tracer=tracer,
1025+
sem_conv_opt_in_mode=sem_conv_opt_in_mode,
1026+
async_request_hook=async_request_hook,
1027+
async_response_hook=async_response_hook,
1028+
),
9901029
)
9911030
for transport in client._mounts.values():
9921031
# FIXME: check it's not wrapped already?
9931032
wrap_function_wrapper(
9941033
transport,
9951034
"handle_async_request",
996-
self._handle_async_request_wrapper,
1035+
partial(
1036+
self._handle_async_request_wrapper,
1037+
tracer=tracer,
1038+
sem_conv_opt_in_mode=sem_conv_opt_in_mode,
1039+
async_request_hook=async_request_hook,
1040+
async_response_hook=async_response_hook,
1041+
),
9971042
)
9981043
client._is_instrumented_by_opentelemetry = True
9991044

0 commit comments

Comments
 (0)