fix issue in opentracing shim causing span to be wrapped unnecessarily

[codeboten]

Description

The OpenTracing shim was unnecessarily wrapping a valid span in a NonRecordingSpan. This was done because of an assumption that the span was a SpanContext. Added a check to ensure only SpanContext are wrapped

Fixes #1166

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Tested w/ the code provided in the issue, updated to account for changed package names/paths

import requests
from opentracing import Format
from opentracing.ext import tags


def load_jaeger(host, port, service_name="testing-jaeger"):
    from jaeger_client.config import Config
    config = Config(
        config={
            "sampler": {
                "type": "const",
                "param": 1,
            },
            "local_agent": {
                "reporting_host": host,
                "reporting_port": port,
            },
            "logging": True,
            "reporter_batch_size": 1,
        },
        service_name=service_name,
    )
    return config.new_tracer()


def load_shim(host, port, service_name="testing-shim"):
    from opentelemetry import trace
    from opentelemetry.exporter.jaeger.thrift import JaegerExporter
    from opentelemetry.shim import opentracing_shim
    from opentelemetry.sdk.trace import TracerProvider
    from opentelemetry.sdk.trace.export import SimpleSpanProcessor
    # Configure the tracer using the default implementation
    trace.set_tracer_provider(TracerProvider())
    tracer_provider = trace.get_tracer_provider()

    # Configure the tracer to export traces to Jaeger
    jaeger_exporter = JaegerExporter(
        agent_host_name=host,
        agent_port=port,
    )
    span_processor = SimpleSpanProcessor(jaeger_exporter)
    tracer_provider.add_span_processor(span_processor)

    # Create an OpenTracing shim. This implements the OpenTracing tracer API, but
    # forwards calls to the underlying OpenTelemetry tracer.
    return opentracing_shim.create_tracer(tracer_provider)


def get_tracer(use_shim):
    host = 'localhost'
    port = 6831
    if use_shim:
        return load_shim(host, port)
    else:
        return load_jaeger(host, port)


# imagine this is also used by an additional logging handler
def add_log(event, data=None):
    if tracer.active_span:
        if not data:
            data = {}
        data["event"] = event
        tracer.active_span.log_kv(data)

def init_trace(request):
    span_ctx = tracer.extract(Format.HTTP_HEADERS, request.headers)
    span_tags = {tags.SPAN_KIND: tags.SPAN_KIND_RPC_SERVER}
    tracer.start_active_span("test", child_of=span_ctx, tags=span_tags)
    add_log("start")


def add_headers_for_trace(method, url, headers):
    if tracer.active_span:
        add_log("call", {"method": method, "url": url})
        span = tracer.active_span
        span.set_tag(tags.HTTP_METHOD, method)
        span.set_tag(tags.HTTP_URL, url)
        span.set_tag(tags.SPAN_KIND, tags.SPAN_KIND_RPC_CLIENT)
        tracer.inject(span, Format.HTTP_HEADERS, headers)  # fails with shim
    return headers


def complete_trace():
    if tracer.active_span:
        add_log("complete")
        tracer.active_span.finish()


def run_trace_example(request):
    init_trace(request)
    add_log("log", {"message": "Hello"})
    method = "GET"
    url = "http://example.com/"  # pretending this to be a real service we're calling
    headers = add_headers_for_trace(method, url, {"content-type": "application/json"})
    r = requests.request(method, url, headers=headers)
    add_log("response", {"status": r.status_code})
    complete_trace()


dummy_request = lambda x: x
setattr(dummy_request, "headers", {"content-type": "application/json"})

# This works perfectly with the Jaeger Client
tracer = get_tracer(False)
run_trace_example(dummy_request)
tracer.close()

# remove the tracer and start over
del tracer

# This fails with the Shim.
tracer = get_tracer(True)
run_trace_example(dummy_request)
tracer.close()

Does This PR Require a Contrib Repo Change?

• No.

Checklist:

• Followed the style guidelines of this project
• Changelogs have been updated
• Unit tests have been added
• Documentation has been updated