Skip to content

Remove OpenTelemetry class #157

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

Merged
merged 2 commits into from
Mar 26, 2024
Merged

Conversation

pquentin
Copy link
Member

Since deserialization happens in the main client, this is the place where the OpenTelemetry class and context manager should live.

Since deserialization happens in the main client, this is the place
where the OpenTelemetry class and context manager should live.
@@ -174,7 +174,7 @@ def __init__(
# sniffing. Uses '_sniffing_task' instead.
self._sniffing_lock = None # type: ignore[assignment]

async def perform_request( # type: ignore[override]
Copy link
Contributor

@miguelgrinberg miguelgrinberg Mar 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume we do not care that the public interface of this method changes? This is called only by the client library package (which we are also updating in parallel)? Any other possible callers that may break with this change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general we do care about the public interface, which is used by projects like https://github.com/elastic/enterprise-search-python, https://github.com/elastic/elasticsearch-serverless-python and https://github.com/ywangd/peek/. But:

  • We don't care about removing endpoint_id and path_parts from the signature because this change was never released.
  • We don't care about adding otel_span because it is a new keyword argument with a default value, and thus not breaking.

@pquentin pquentin merged commit 7e76f8d into elastic:main Mar 26, 2024
@pquentin pquentin deleted the otel-elasticsearch-py branch September 17, 2024 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants