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

unparented segment/subsegment resolution #67

Closed
thehesiod opened this issue Jun 7, 2018 · 5 comments
Closed

unparented segment/subsegment resolution #67

thehesiod opened this issue Jun 7, 2018 · 5 comments

Comments

@thehesiod
Copy link
Contributor

thehesiod commented Jun 7, 2018

I'm using aws-xray-sdk to trace an application which uses the google pubsub module. Unfortunately this module uses various threads which means that I get a lot of SegmentNotFoundException exceptions causing my application to fail.

It's going to be impossible for me to track down all the locations where it creates threads and pass the trace entity and call xray_recorder.set_trace_entity.

How about if this SDK provides support for a "root thread" which it will use to resolve the segment/subsegment if the current context does not have one?

@haotianw465
Copy link
Contributor

Hi, sorry for the late response. I saw the UnparentedContext in your PR and this could definitely be an option for context management. But before going further to simply add that, I would like to know more about your exception on tracing under pubsub module.

Here are several options:

  1. Set context missing strategy to LOR_ERROR so SegmentNotFoundException won't be thrown. This way you will lose tracing data that doesn't have a context.
  2. Like what you did with the UnparentedContext. This way each unparented segment carries a different trace id so you won't be able to easily correlates tracing data sent from various thread. Maybe you are doing some extra work in addition to what you actually pasted in the PR comment?
  3. We have an item on our backlog for supporting automatic context propagation from main thread to worker threads with future.ThreadPoolExecutor. The downside is that this only fits very limited use case and for some libraries like s3transfer you have to go very deep to figure out what threading module it is using.

Please let me know in an ideal case how you expect your tracing data to be structured so I can provide further help.

@thehesiod thehesiod changed the title root thread for segment/subsegment resolution unparented segment/subsegment resolution Jun 7, 2018
@thehesiod
Copy link
Contributor Author

late? that was really fast :)

  1. Ya that's an existing option, however we lose these traces like you stated
  2. That's what I'm trying now, we'll see how messy it gets :) Basically the google pubsub client library creates a bunch of background helpers which contact the network and I want to get some visibility into what they're doing. Unfortunately when they run I have no active "root" thread so they're completely isolated without any hierarchy. In my scenario I'm using the apscheduler, and while my callback is inactive these threads are firing off network calls.
  3. That would be nice but there are cases where the workers do work w/o the thread which owns the pool having an active segment (like my case).

I think it's worth discussing what the ideal solution would be, I have no firm proposal.

@haotianw465
Copy link
Contributor

Hi,

Please let me know if solution2 works for you. In that case I would imagine your service graph is split into two parts. One represents the request flow starts from your main thread and the other one represents the background work. All background work subsegments will be aggregated to a single node with name like unparented segment and you still see errors, outbound calls etc if applicable.

You can also do some more complicated logic on how to create the unparented segment so you categorize the workers into different logical nodes on the service graph to fit your need.

Please let me know your findings and I can see a general enough unparented context being added to the SDK built-in context.

@thehesiod
Copy link
Contributor Author

ya solution 2 seems to be working for me. Right now I have them all in one unparented bucket, but I could imagine some people may want them categorized by thread or perhaps pool (which could include a name for more context). However I decided not to do that because the threads may be one-offs.

@haotianw465
Copy link
Contributor

I'm closing this issue now. Will put the UnparentedContext as a backlog and prioritize it accordingly should more customers request this specific use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants