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

Help Extracting Distributed Trace #654

Closed
jon-whit opened this issue May 6, 2020 · 4 comments
Closed

Help Extracting Distributed Trace #654

jon-whit opened this issue May 6, 2020 · 4 comments

Comments

@jon-whit
Copy link

jon-whit commented May 6, 2020

I'm using the Python Falcon framework and I'm trying to implement a middleware to create a span for the HTTP handlers being invoked.

Here's more information on middlewares in the Falcon framework.

Here's the code I'm dealing with..

// tracer.py
from opentelemetry import trace
from opentelemetry.sdk.trace import TracerProvider
from opentelemetry.trace.propagation import (
    get_span_from_context,
    set_span_in_context,
)
from opentelemetry.trace.propagation.tracecontexthttptextformat import TraceContextHTTPTextFormat
from opentelemetry.ext import jaeger
from opentelemetry.sdk.trace.export import BatchExportSpanProcessor
from falcon import Request, Response

trace.set_tracer_provider(TracerProvider())

jaeger_exporter = jaeger.JaegerSpanExporter(
    service_name='service1',
    agent_host_name='localhost',
    agent_port=6831,
)

span_processor = BatchExportSpanProcessor(jaeger_exporter)
trace.get_tracer_provider().add_span_processor(
    span_processor
)

tracer = trace.get_tracer(__name__)

def getTraceParent(headers, key):
    h = dict((k.lower(), v) for k, v in headers.items())
    return [h[k] for k in h if k == key]

class HTTPTraceMiddleware(object):
    def __init__(self, tracer, attributes, trace_headers=False):
        self.tracer = tracer
        self.attributes = attributes

    def process_request(self, req: Request, resp: Response) -> None:

        # Extract the Trace context from the HTTP request header (if it exists)
        propagator = TraceContextHTTPTextFormat()
        ctx = propagator.extract(get_from_carrier=getTraceParent, carrier=req.headers)
        req.context = ctx
        
        s = get_span_from_context(ctx)
        parent = None
        if s.get_context().is_valid():
            parent = s
        
        span = tracer.start_span(name="falcon.trace", attributes=self.attributes, parent=parent)
        span.set_attribute("http.method", req.method)
        span.set_attribute("http.url", req.url)

        set_span_in_context(span, req.context)

    def process_resource(self, req, resp, resource, params):
        span = get_span_from_context(req.context)
        span.resource = '%s %s' % (req.method, _name(resource))

    def process_response(self, req, resp, resource, req_succeeded=None):
        span = get_span_from_context(req.context)
        span.set_attribute("http.status", resp.status)
        span.end()

def _name(r):
    return '%s.%s' % (r.__module__, r.__class__.__name__)

Based on my debugging I'm not getting the context set and retrieved the proper way. I'm trying to use explicit propagation (coming from a Go background). How do I achieve this with the opentelemetry-python API? I'm at a loss with the documentation. I've been trying to get this to work for a while now..

In Jaeger I don't see the trace at all, which tells me span.end() isn't getting called on the span created in the process_request method. What's the proper way to get/set the context like I'm trying to do here?

@jon-whit jon-whit changed the title Help Extracting and Propagating Distributed Trace Help Extracting Distributed Trace May 6, 2020
@codeboten
Copy link
Contributor

Hey @jon-whit, thanks for opening the issue. I think the code you have is actually really close, the only thing I changed to get it working was to update req.context at the end of process_request to the following:

req.context = set_span_in_context(span, req.context)

Any action that sets a value on a context returns a new copy of the context with an updated value.
I also updated the code to use a ConsoleSpanExporter to make troubleshooting easier:

console_exporter = ConsoleSpanExporter()
span_processor = BatchExportSpanProcessor(console_exporter)
trace.get_tracer_provider().add_span_processor(span_processor)

Let me know if you have any other questions!

It would be awesome if you wanted to submit a PR to provide support to the falcon middleware as part of the project.

@jon-whit
Copy link
Author

jon-whit commented May 6, 2020

@codeboten awesome! That worked like a charm. Good to know 👍

I'd be happy to submit a PR to provide a middleware for Falcon. Where would I make most of those changes? Would that be to the ext directory? Any examples would be nice.

@jon-whit jon-whit closed this as completed May 6, 2020
@codeboten
Copy link
Contributor

@jon-whit great to hear!

Yes, the ext directory is where the code would live. You can look at any of the instrumentations in there to give you an idea for what the format looks like. You can see an example of a PR to add new instrumentation here: https://github.com/open-telemetry/opentelemetry-python/pull/643/files

It might also be worth looking at the code in this repo to see if there are any cases or tests that you haven't addressed yet in your code: https://github.com/open-telemetry/opentelemetry-python-contrib/tree/master/reference/ddtrace/contrib/falcon.

NOTE: The contrib repository contains code that has not yet been ported to use OpenTelemetry, it's code that was donated by DataDog and that is currently in process of being ported over (one instrumentation at a time)

@jon-whit
Copy link
Author

jon-whit commented May 6, 2020

@codeboten thank you so much! I'll look into getting some contributions in for Falcon. I'll have to do it on my own time, outside of primary work business hours 👍

srikanthccv pushed a commit to srikanthccv/opentelemetry-python that referenced this issue Nov 1, 2020
…-telemetry#636 (open-telemetry#654)

* feat: warn user when a instrumented package was already required open-telemetry#636

* chore: address PR comments

* chore: extract package name from require.cache

* chore: use find instead of some

* chore: use require.resolve to find already required modules

* chore: try/catch require.resolve

Co-authored-by: Mayur Kale <[email protected]>
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

No branches or pull requests

2 participants