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

CP-50270: Set the correct parent in make_connection #5834

Merged

Conversation

GabrielBuica
Copy link
Contributor

Updates the correct parent span when we retrieve a cached connection.

Preveusly, we would call the instrumentation only when we make the connection for the first time. This resulted in seeing the same parent span for all subsequent calls of make_connection. Calling the instrumentation first ensures the correct parent is sent over the headers for all code branches.

Updates the correct parent span when we retrieve a cached connection.

Preveusly, we would call the instrumentation only when we make the
connection for the first time. This resulted in seeing the same parent
span for all subsequent calls of `make_connection`.
Calling the instrumentation first ensures the correct parent is sent
over the headers for all code branches.

Signed-off-by: Gabriel Buica <danutgabriel.buica@cloud.com>
Copy link
Contributor

@edwintorok edwintorok left a comment

Choose a reason for hiding this comment

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

This probably got lost/broken inbetween refactorings on previous PRs.

In the original code that I used for my experiments it looked like this, which matches the order in this PR:

        from opentelemetry import trace, propagators, baggage
        from opentelemetry.trace.propagation.tracecontext import TraceContextTextMapPropagator
        from opentelemetry.baggage.propagation import W3CBaggagePropagator
        from opentelemetry.sdk.trace import TracerProvider
        ctx = baggage.set_baggage("hello", "world")

        headers = {}
        W3CBaggagePropagator().inject(headers, ctx)
        TraceContextTextMapPropagator().inject(headers, ctx)

        self._extra_headers = []
        for k,v in headers.items():
           self.add_extra_header(k, v)

        # compatibility with parent xmlrpclib.Transport HTTP/1.1 support
        if self._connection and host == self._connection[0]:
            return self._connection[1]

        self._connection = host, UDSHTTPConnection(host)
        return self._connection[1]

The important part is to also have self._extra_headers = [] (otherwise you accumulate a new header every time, and eventually you exhaust max header size), but we have that already too.

@edwintorok edwintorok merged commit fbcc033 into xapi-project:master Jul 15, 2024
15 checks passed
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.

3 participants