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

bugfix: correct generate es search span_name #1018

Merged
merged 2 commits into from
Mar 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#999])(https://github.com/open-telemetry/opentelemetry-python-contrib/pull/999)
- `opentelemetry-instrumentation-falcon` Falcon: Capture custom request/response headers in span attributes
([#1003])(https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1003)
- `opentelemetry-instrumentation-elasticsearch` no longer creates unique span names by including search target, replaces them with `<target>` and puts the value in attribute `elasticsearch.target`
([#1018](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1018))

## [1.10.0-0.29b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.10.0-0.29b0) - 2022-03-10

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,11 +150,14 @@ def _uninstrument(self, **kwargs):

_regex_doc_url = re.compile(r"/_doc/([^/]+)")

# search api https://www.elastic.co/guide/en/elasticsearch/reference/current/search-search.html
_regex_search_url = re.compile(r"/([^/]+)/_search[/]?")


def _wrap_perform_request(
tracer, span_name_prefix, request_hook=None, response_hook=None
):
# pylint: disable=R0912
# pylint: disable=R0912,R0914
def wrapper(wrapped, _, args, kwargs):
method = url = None
try:
Expand All @@ -167,7 +170,10 @@ def wrapper(wrapped, _, args, kwargs):
)

op_name = span_name_prefix + (url or method or _DEFAULT_OP_NAME)

doc_id = None
search_target = None

if url:
# TODO: This regex-based solution avoids creating an unbounded number of span names, but should be replaced by instrumenting individual Elasticsearch methods instead of Transport.perform_request()
# A limitation of the regex is that only the '_doc' mapping type is supported. Mapping types are deprecated since Elasticsearch 7
Expand All @@ -184,6 +190,11 @@ def wrapper(wrapped, _, args, kwargs):
)
# Put the document ID in attributes
doc_id = match.group(1)
match = _regex_search_url.search(url)
if match is not None:
op_name = span_name_prefix + "/<target>/_search"
search_target = match.group(1)

params = kwargs.get("params", {})
body = kwargs.get("body", None)

Expand All @@ -209,6 +220,8 @@ def wrapper(wrapped, _, args, kwargs):
attributes["elasticsearch.params"] = str(params)
if doc_id:
attributes["elasticsearch.id"] = doc_id
if search_target:
attributes["elasticsearch.target"] = search_target
for key, value in attributes.items():
span.set_attribute(key, value)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,14 +237,15 @@ def test_dsl_search(self, request_mock):
spans = self.get_finished_spans()
span = spans[0]
self.assertEqual(1, len(spans))
self.assertEqual(span.name, "Elasticsearch/test-index/_search")
self.assertEqual(span.name, "Elasticsearch/<target>/_search")
self.assertIsNotNone(span.end_time)
self.assertEqual(
span.attributes,
{
SpanAttributes.DB_SYSTEM: "elasticsearch",
"elasticsearch.url": "/test-index/_search",
"elasticsearch.method": helpers.dsl_search_method,
"elasticsearch.target": "test-index",
SpanAttributes.DB_STATEMENT: str(
{
"query": {
Expand Down