Support for generic annotations and meta for third-party libraries about aws-xray-sdk-python HOT 5 OPEN

I'm not sure if I understand your question. Whenever the annotation is available you can always use

request_id = grab_id()
current = xray_recorder.get_trace_entity()
current.put_annotation('key', request_id)

Could you clarify your use case a little bit on the aiohttp case on when and where you want to annotate?

from aws-xray-sdk-python.

Your proposal is good but makes the solution to much complicate IMO, let me give an example that uses the Aiohttp extensions to trace incoming requests and downstream calls:

Let's say that I want to add a new annotation to each trace that helps us to identify univocally all of the traces - segments + subsegments - that belong to a specific request. For that we have a header, let's call it X-Request-id, that is passed through all of the services. So a user that makes a request to one of our frontend services will send this header and this will be used to send it to all of the services that are used to complete the user query.

So, having this value in the AWS Xray trace registry this allows us to correlate the traces with our logs for all of the requests that were sampled.

So in the case of the Aiohttp, we have to make something like:

from aws_xray_sdk.ext.aiohttp.middleware import middleware

async def requets_id_annotation(request, handler)
    """
    This  middleware is only used to save the request_id
    as a new AWS Xray annotation value for the ongoing
    trace segment
    """
    request_id = request.headers['X-Request-id']
    current = xray_recorder.get_trace_entity()
    current.put_annotation('key', request_id)
    return await handler(request)

app = web.Application(loop=loop, middlewares=[request_id_annotation, middeware])

As you can see, we are generating just an ad-hoc middleware which the purpose is just to grab the request id and save it to the opened segment. Take into account that this solution implies:

1. Call per each request a new middleware, with the performance issues that it implies.
2. Use the concept of the right to left middleware execution that allows us to add a new middleware just after the official AWS Xray one, having the requets_id_annotation in the right position to get the opened segment.

For tracing the downstream calls the pattern is more or less the same, perhaps:

ClientSession(trace_configs=[aws_xray_trace_config(), requets_id_annotation_config()])

Repeating the same pattern of the middleware with the same issues already commented.

So how can we improve this situation to reduce the friction and code verbosity? The idea is to give to the developer a generic interface that would allow amending the traces that are produced during the execution of the program, perhaps:

def hook_meta(segment):
   segment.add_meta(....)

def hook_annotations(segment):
   segment.add_annotation(....)

xray_recorder.configure(....., hook_meta, hook_annotations)

With this generics hooks the developer has an interface that would allow correlating generic meta or annotations for all of the segments and sub-segments that are produced during the execution of the program.

from aws-xray-sdk-python.

Sorry for the late response. One thing I would like to clarify is that the X-Ray service aggregates metadata and annotations to trace level. In the aiohttp case all segments/subsegments generated for a single incoming request are already correlated with a trace id, so as long as you add the request_id to any segment/subsegment, you can then filter on that request id and see all segments/subsegments for that request. IMO a recorder level hook is not necessary as the recorder will blindly add the pre-defined metadata/annotations to each trace entity, which increases unnecessary payload and adds more overhead.

Having said that, I do think there should be a more convenient way to add metadata/annotations to X-Ray provided middleware or library patcher, to make user code less verbose. I propose some callback that you register on a per middleware/library basis. In the aiohttp case you will do something like

def my_cb(request, entity):
    id = request.headers['x-request-id']
    entity.put_annotation('id', id)
    metadata = generate_whatever_i_need()
    entity.put_metadata(metadata)

xray_recorder.register_callback('aiohttp', my_cb)

Then in the X-Ray aiohttp middleware it will do some extra work

your_cb = xray_recorder.get_cb('aiohttp')
your_cb(request, segment)

This way any aiohttp user can register any callback and extract whatever extra information they want and add adds annotations/metadata. We can do similar thing on third party library patcher to have the recorder to do some customized work to add additional info when doing capture work.

But some explicit contract is necessary so the recorder knows what parameter to pass to the user defined callback and the users know what parameters are expected to be provided when writing the callback.

Please let me know your thoughts.

from aws-xray-sdk-python.

Thanks for the clarification, good to know that is only needed to add the metadata in one place.

Regarding your proposal I like it, and more important solves the issue. What I don't like is having the callback configured at recorder level and being used explicitly only for the third integrations, it's kinda weird IMO.

So what I would propose is move the callback registration as close as we can to the third party package, this will allow us to implement the callback per integration, perhaps in the case of the Aiohttp middleware:

from aws_xray_sdk.ext.aiohttp.middleware import middleware, register_cb

def cb(request, entity):
    entity.put_annotation('id', request.headers['id'])

register_cb(cb)
app = web.Application(middlewares=[middleware])

Thoughts?

from aws-xray-sdk-python.

This is a good idea. I agree with this approach. I will put this on our backlog and get it out as soon as possible. A PR is always welcome.

from aws-xray-sdk-python.