Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

opentracer-shim not fully compatible with opentracing #1166

Closed
Tecktron opened this issue Sep 25, 2020 · 3 comments · Fixed by #1776
Closed

opentracer-shim not fully compatible with opentracing #1166

Tecktron opened this issue Sep 25, 2020 · 3 comments · Fixed by #1776
Assignees
Labels
bug Something isn't working shim OpenTracing or OpenCensus compatibility triaged

Comments

@Tecktron
Copy link

Tecktron commented Sep 25, 2020

This is a bit of a case study. The current implementation of jaeger-client-python doesn't support W3C trace headers, which some newer services in our stack not written in python are now using (via opencensus), thus we need to become compatible.

Having taken note that the opentracing & opencensus packages are (supposedly) merging into this package, I've been making an effort to update some existing python code bases. I stumbled upon the opentracer-shim in this project in hopes that I could easily swap out the loading of jaeger-client tracer and be good to go.

However, I've found that it isn't actually compatible in the same way. I've written a brief example to illustrate the issue which is based on a combination of my code, the opentracer examples and the shim example.

While true that I might just be being lazy and should probably just either go all in and refactor everything to use this package or help update the jaeger client (still making that choice), I wanted to raise the issue that the shim is not compatible with this basic example (as I was hoping it would be) without making some major changes in the code, making the shim not so much a shim.

Please run this example to see the breaking point.
Hint: it's the inject call that fails directly with a AttributeError: 'Span' object has no attribute 'trace_id', but seems to stem from a deeper architectural incompatibility.

requirements

pip install requests jaeger-client opentracing opentelemetry-api opentelemetry-sdk opentelemetry-exporter-jaeger opentelemetry-opentracing-shim

example.py

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 import JaegerSpanExporter
    from opentelemetry.instrumentation import opentracing_shim
    from opentelemetry.sdk.trace import TracerProvider
    from opentelemetry.sdk.trace.export import SimpleExportSpanProcessor
    # 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 = JaegerSpanExporter(
        service_name=service_name,
        agent_host_name=host,
        agent_port=port,
    )
    span_processor = SimpleExportSpanProcessor(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()

Any thoughts or ideas appreciated. Thanks!

@Tecktron Tecktron added the bug Something isn't working label Sep 25, 2020
@HaddadJoe
Copy link

HaddadJoe commented Sep 28, 2020

We are having the same issue described above.

After a quick look, I think it might be due to how the inject function is implemented in the shim extension

ctx = set_span_in_context(DefaultSpan(span_context.unwrap()))

Shouldn't it be creating a SpanShim ?

@codeboten codeboten added the shim OpenTracing or OpenCensus compatibility label Nov 26, 2020
@codeboten codeboten self-assigned this Nov 26, 2020
@github-actions
Copy link

github-actions bot commented Apr 9, 2021

This issue was marked stale due to lack of activity. It will be closed in 30 days.

@Tecktron
Copy link
Author

Bump, this is still an issue.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working shim OpenTracing or OpenCensus compatibility triaged
Projects
None yet
3 participants