feat: Add elasticsearch instrumentation#882
Conversation
CagriYonca
commented
Jun 18, 2026
- Added elasticsearch instrumentation.
|
❌ @CagriYonca the
📝 What should I do to fix it?All proposed commits should include a sign-off in their messages, ideally at the end. ❔ Why it is requiredThe Developer Certificate of Origin (DCO) is a lightweight way for contributors to certify that they wrote or otherwise have the right to submit the code they are contributing to the project. Here is the full text of the DCO, reformatted for readability:
Contributors sign-off that they adhere to these requirements by adding a Git even has a |
99fcfab to
e9c685e
Compare
|
|
||
| # Connection cache to avoid repeated URL parsing and store cluster info | ||
| # Structure: {connection_id: {host, port, cluster_name, last_updated}} | ||
| _connection_cache: dict[str, dict[str, Any]] = {} |
There was a problem hiding this comment.
Suggestion. You can use defaultdict to create _connection_cache, then you can better manage the usage of the dictionary. But remember, if asking for a missing key _connection_cache, it will return an empty dictionary. WDYT?
| _connection_cache: dict[str, dict[str, Any]] = {} | |
| _connection_cache: dict[str, dict[str, Any]] = defaultdict(dict) |
| if connection_id in _connection_cache: | ||
| cached = _connection_cache[connection_id] | ||
| cluster_name = cached.get("cluster_name") | ||
| if ( | ||
| cluster_name | ||
| and (time.time() - cached.get("last_updated", 0)) | ||
| < CLUSTER_NAME_CACHE_TTL | ||
| ): | ||
| return cluster_name |
There was a problem hiding this comment.
By using defaultdict as suggested before, you can reduce this code to:
| if connection_id in _connection_cache: | |
| cached = _connection_cache[connection_id] | |
| cluster_name = cached.get("cluster_name") | |
| if ( | |
| cluster_name | |
| and (time.time() - cached.get("last_updated", 0)) | |
| < CLUSTER_NAME_CACHE_TTL | |
| ): | |
| return cluster_name | |
| if cached := _connection_cache[connection_id]: | |
| if (cluster_name := cached.get("cluster_name")) and (time.time() - cached.get("last_updated", 0) < CLUSTER_NAME_CACHE_TTL: | |
| return cluster_name |
There was a problem hiding this comment.
Doesn't the walrus operator make it harder to read?
| cached = _get_cached_cluster_name(connection_id) | ||
| if cached: | ||
| return cached |
There was a problem hiding this comment.
| cached = _get_cached_cluster_name(connection_id) | |
| if cached: | |
| return cached | |
| if cached := _get_cached_cluster_name(connection_id): | |
| return cached |
| cluster_name = _extract_cluster_name_from_response(instance.info()) | ||
| if cluster_name: |
There was a problem hiding this comment.
| cluster_name = _extract_cluster_name_from_response(instance.info()) | |
| if cluster_name: | |
| if cluster_name := _extract_cluster_name_from_response(instance.info()) |
| url_lower = url.lower() | ||
|
|
||
| # Multi-operations | ||
| if "_msearch" in url_lower: | ||
| return "msearch" | ||
| if "_mget" in url_lower: | ||
| return "mget" | ||
| if "_bulk" in url_lower: | ||
| return "bulk" | ||
|
|
||
| # Search operations | ||
| if "_search" in url_lower: | ||
| return "search" | ||
|
|
There was a problem hiding this comment.
SonarCloud reported a Cognitive Complexity in this function due to the many if statements. Why not use something like:
| url_lower = url.lower() | |
| # Multi-operations | |
| if "_msearch" in url_lower: | |
| return "msearch" | |
| if "_mget" in url_lower: | |
| return "mget" | |
| if "_bulk" in url_lower: | |
| return "bulk" | |
| # Search operations | |
| if "_search" in url_lower: | |
| return "search" | |
| url_lower = url.lower() | |
| if url_lower.startswith("_"): | |
| return url_lower[1,] |
| # Response status code | ||
| if hasattr(response, "meta") and hasattr(response.meta, "status"): | ||
| status_code = response.meta.status | ||
| span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, status_code) |
There was a problem hiding this comment.
Why do we set the HTTP status code attribute since this is a DB-type operation?
There was a problem hiding this comment.
You're right, it uses urllib3 to make DB calls but semantically DB span shouldn't have HTTP_STATUS_CODE. It's been removed
e9c685e to
e9b97d8
Compare
Signed-off-by: Cagri Yonca <cagri@ibm.com>
eb46efa to
4b3de8a
Compare
Signed-off-by: Cagri Yonca <cagri@ibm.com>
4b3de8a to
b7d765a
Compare
|


