Skip to content

Commit

Permalink
Merge branch 'main' into bugfix/registered-method-error
Browse files Browse the repository at this point in the history
  • Loading branch information
tammy-baylis-swi authored May 17, 2024
2 parents 1e87c88 + f8758c6 commit 68faaab
Show file tree
Hide file tree
Showing 33 changed files with 605 additions and 151 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#2418](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2418))
- Use sqlalchemy version in sqlalchemy commenter instead of opentelemetry library version
([#2404](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2404))
- `opentelemetry-instrumentation-asyncio` Check for cancelledException in the future
([#2461](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2461))
- Remove SDK dependency from opentelemetry-instrumentation-grpc
([#2474](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2474))
- `opentelemetry-instrumentation-elasticsearch` Improved support for version 8
([#2420](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2420))

## Version 1.24.0/0.45b0 (2024-03-28)

Expand Down
4 changes: 2 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ You can run `tox` with the following arguments:
`black` and `isort` are executed when `tox -e lint` is run. The reported errors can be tedious to fix manually.
An easier way to do so is:

1. Run `.tox/lint-some-package/bin/black .`
2. Run `.tox/lint-some-package/bin/isort .`
1. Run `.tox/lint/bin/black .`
2. Run `.tox/lint/bin/isort .`

Or you can call formatting and linting in one command by [pre-commit](https://pre-commit.com/):

Expand Down
2 changes: 1 addition & 1 deletion gen-requirements.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
-c dev-requirements.txt
astor==0.8.1
jinja2==3.1.3
jinja2==3.1.4
markupsafe==2.0.1
isort
black
Expand Down
2 changes: 1 addition & 1 deletion instrumentation/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
| [opentelemetry-instrumentation-confluent-kafka](./opentelemetry-instrumentation-confluent-kafka) | confluent-kafka >= 1.8.2, <= 2.3.0 | No | experimental
| [opentelemetry-instrumentation-dbapi](./opentelemetry-instrumentation-dbapi) | dbapi | No | experimental
| [opentelemetry-instrumentation-django](./opentelemetry-instrumentation-django) | django >= 1.10 | Yes | experimental
| [opentelemetry-instrumentation-elasticsearch](./opentelemetry-instrumentation-elasticsearch) | elasticsearch >= 2.0 | No | 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-flask](./opentelemetry-instrumentation-flask) | flask >= 1.0 | Yes | migration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,21 +116,11 @@ class AsyncioInstrumentor(BaseInstrumentor):
"run_coroutine_threadsafe",
]

def __init__(self):
super().__init__()
self.process_duration_histogram = None
self.process_created_counter = None

self._tracer = None
self._meter = None
self._coros_name_to_trace: set = set()
self._to_thread_name_to_trace: set = set()
self._future_active_enabled: bool = False

def instrumentation_dependencies(self) -> Collection[str]:
return _instruments

def _instrument(self, **kwargs):
# pylint: disable=attribute-defined-outside-init
self._tracer = get_tracer(
__name__, __version__, kwargs.get("tracer_provider")
)
Expand Down Expand Up @@ -307,13 +297,17 @@ def trace_future(self, future):
)

def callback(f):
exception = f.exception()
attr = {
"type": "future",
"state": (
"cancelled"
if f.cancelled()
else determine_state(f.exception())
),
}
state = determine_state(exception)
attr["state"] = state
self.record_process(start, attr, span, exception)
self.record_process(
start, attr, span, None if f.cancelled() else f.exception()
)

future.add_done_callback(callback)
return future
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import asyncio
from unittest.mock import patch

from opentelemetry.instrumentation.asyncio import AsyncioInstrumentor
from opentelemetry.instrumentation.asyncio.environment_variables import (
OTEL_PYTHON_ASYNCIO_FUTURE_TRACE_ENABLED,
)
from opentelemetry.test.test_base import TestBase
from opentelemetry.trace import get_tracer


class TestTraceFuture(TestBase):
@patch.dict(
"os.environ", {OTEL_PYTHON_ASYNCIO_FUTURE_TRACE_ENABLED: "true"}
)
def setUp(self):
super().setUp()
self._tracer = get_tracer(
__name__,
)
self.instrumentor = AsyncioInstrumentor()
self.instrumentor.instrument()

def tearDown(self):
super().tearDown()
self.instrumentor.uninstrument()

def test_trace_future_cancelled(self):
async def future_cancelled():
with self._tracer.start_as_current_span("root"):
future = asyncio.Future()
future = self.instrumentor.trace_future(future)
future.cancel()

try:
asyncio.run(future_cancelled())
except asyncio.CancelledError as exc:
self.assertEqual(isinstance(exc, asyncio.CancelledError), True)
spans = self.memory_exporter.get_finished_spans()
self.assertEqual(len(spans), 2)
self.assertEqual(spans[0].name, "root")
self.assertEqual(spans[1].name, "asyncio future")

metrics = (
self.memory_metrics_reader.get_metrics_data()
.resource_metrics[0]
.scope_metrics[0]
.metrics
)
self.assertEqual(len(metrics), 2)

self.assertEqual(metrics[0].name, "asyncio.process.duration")
self.assertEqual(
metrics[0].data.data_points[0].attributes["state"], "cancelled"
)

self.assertEqual(metrics[1].name, "asyncio.process.created")
self.assertEqual(
metrics[1].data.data_points[0].attributes["state"], "cancelled"
)
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
request_hook (Callable) - a function with extra user-defined logic to be performed before performing the request
this function signature is: def request_hook(span: Span, service_name: str, operation_name: str, api_params: dict) -> None
response_hook (Callable) - a function with extra user-defined logic to be performed after performing the request
this function signature is: def request_hook(span: Span, service_name: str, operation_name: str, result: dict) -> None
this function signature is: def response_hook(span: Span, service_name: str, operation_name: str, result: dict) -> None
for example:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def response_hook(span, response):
from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
from opentelemetry.instrumentation.utils import unwrap
from opentelemetry.semconv.trace import SpanAttributes
from opentelemetry.trace import SpanKind, get_tracer
from opentelemetry.trace import SpanKind, Status, StatusCode, get_tracer

from .utils import sanitize_body

Expand All @@ -103,6 +103,7 @@ def response_hook(span, response):
es_transport_split = elasticsearch.VERSION[0] > 7
if es_transport_split:
import elastic_transport
from elastic_transport._models import DefaultType

logger = getLogger(__name__)

Expand Down Expand Up @@ -173,7 +174,12 @@ def _instrument(self, **kwargs):

def _uninstrument(self, **kwargs):
# pylint: disable=no-member
unwrap(elasticsearch.Transport, "perform_request")
transport_class = (
elastic_transport.Transport
if es_transport_split
else elasticsearch.Transport
)
unwrap(transport_class, "perform_request")


_regex_doc_url = re.compile(r"/_doc/([^/]+)")
Expand All @@ -182,6 +188,7 @@ def _uninstrument(self, **kwargs):
_regex_search_url = re.compile(r"/([^/]+)/_search[/]?")


# pylint: disable=too-many-statements
def _wrap_perform_request(
tracer,
span_name_prefix,
Expand Down Expand Up @@ -234,7 +241,22 @@ def wrapper(wrapped, _, args, kwargs):
kind=SpanKind.CLIENT,
) as span:
if callable(request_hook):
request_hook(span, method, url, kwargs)
# elasticsearch 8 changed the parameters quite a bit
if es_transport_split:

def normalize_kwargs(k, v):
if isinstance(v, DefaultType):
v = str(v)
elif isinstance(v, elastic_transport.HttpHeaders):
v = dict(v)
return (k, v)

hook_kwargs = dict(
normalize_kwargs(k, v) for k, v in kwargs.items()
)
else:
hook_kwargs = kwargs
request_hook(span, method, url, hook_kwargs)

if span.is_recording():
attributes = {
Expand All @@ -260,16 +282,41 @@ def wrapper(wrapped, _, args, kwargs):
span.set_attribute(key, value)

rv = wrapped(*args, **kwargs)
if isinstance(rv, dict) and span.is_recording():

body = rv.body if es_transport_split else rv
if isinstance(body, dict) and span.is_recording():
for member in _ATTRIBUTES_FROM_RESULT:
if member in rv:
if member in body:
span.set_attribute(
f"elasticsearch.{member}",
str(rv[member]),
str(body[member]),
)

# since the transport split the raising of exceptions that set the error status
# are called after this code so need to set error status manually
if es_transport_split and span.is_recording():
if not (method == "HEAD" and rv.meta.status == 404) and (
not 200 <= rv.meta.status < 299
):
exception = elasticsearch.exceptions.HTTP_EXCEPTIONS.get(
rv.meta.status, elasticsearch.exceptions.ApiError
)
message = str(body)
if isinstance(body, dict):
error = body.get("error", message)
if isinstance(error, dict) and "type" in error:
error = error["type"]
message = error

span.set_status(
Status(
status_code=StatusCode.ERROR,
description=f"{exception.__name__}: {message}",
)
)

if callable(response_hook):
response_hook(span, rv)
response_hook(span, body)
return rv

return wrapper
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@
# limitations under the License.


_instruments = ("elasticsearch >= 2.0",)
_instruments = ("elasticsearch >= 6.0",)
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
asgiref==3.7.2
attrs==23.2.0
Deprecated==1.2.14
elasticsearch==8.12.1
elasticsearch-dsl==8.12.0
elastic-transport==8.12.0
importlib-metadata==6.11.0
iniconfig==2.0.0
packaging==23.2
pluggy==1.4.0
py==1.11.0
py-cpuinfo==9.0.0
pytest==7.1.3
pytest-benchmark==4.0.0
python-dateutil==2.8.2
six==1.16.0
tomli==2.0.1
typing_extensions==4.10.0
urllib3==2.2.1
wrapt==1.16.0
zipp==3.17.0
-e opentelemetry-instrumentation
-e instrumentation/opentelemetry-instrumentation-elasticsearch
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,9 @@ class Index:
dsl_index_span_name = "Elasticsearch/test-index/doc/2"
dsl_index_url = "/test-index/doc/2"
dsl_search_method = "GET"

perform_request_mock_path = "elasticsearch.connection.http_urllib3.Urllib3HttpConnection.perform_request"


def mock_response(body: str, status_code: int = 200):
return (status_code, {}, body)
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,9 @@ class Index:
dsl_index_span_name = "Elasticsearch/test-index/_doc/:id"
dsl_index_url = "/test-index/_doc/2"
dsl_search_method = "POST"

perform_request_mock_path = "elasticsearch.connection.http_urllib3.Urllib3HttpConnection.perform_request"


def mock_response(body: str, status_code: int = 200):
return (status_code, {}, body)
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from elastic_transport import ApiResponseMeta, HttpHeaders
from elastic_transport._node import NodeApiResponse
from elasticsearch_dsl import Document, Keyword, Text


Expand All @@ -36,6 +38,23 @@ class Index:
}
}
dsl_index_result = (1, {}, '{"result": "created"}')
dsl_index_span_name = "Elasticsearch/test-index/_doc/2"
dsl_index_span_name = "Elasticsearch/test-index/_doc/:id"
dsl_index_url = "/test-index/_doc/2"
dsl_search_method = "POST"

perform_request_mock_path = (
"elastic_transport._node._http_urllib3.Urllib3HttpNode.perform_request"
)


def mock_response(body: str, status_code: int = 200):
return NodeApiResponse(
ApiResponseMeta(
status=status_code,
headers=HttpHeaders({}),
duration=100,
http_version="1.1",
node="node",
),
body.encode(),
)
Loading

0 comments on commit 68faaab

Please sign in to comment.