Add annotations #348
Add annotations #348
Conversation
There was a problem hiding this comment.
Hi!
We already support filtering / searching HTTP attributes by using string keywords. Please see the filter expressions section in the X-Ray developer guide documentation.
You can use http.url BEGINSWITH "https://your.url.com" to achieve the same result for your use case.
Unfortunately not.... (this took 4 weeks to resolve with AWS support) This only works where the root segment has had the metadata. As you'll see from the code when running in a Lambda environment the Xray SDK creates subsegments for each request (which handles the re-use of lambdas) This means... it's currently not possible to filter with http.url on Xray deployed in AWS Lambda. Hence this PR -> this uses the workaround recommended by AWS support to use the annotations, which are filterable on subsegments. While I agree that filtering on http.url would be the ideal solution, in the absence of a change in the way Xray filtering is implemented, this workaround resolves the problem of "I can't filter at all" for anyone deploying in a Lambda environment, while leaving the http.url filtering unchanged for those deploying outside of Lambda. |
|
I see, that is definitely a valid point and a change we will consider. However, since X-Ray imposes a limit of 50 annotations per trace, it is important that adding these annotations is an opt-in feature as not to use up the allocated amount for others who may not necessarily need it for this use case. This can be done by adding a new configuration to the Django middleware. You can find more information on how to add one here. |
Thanks, that's a good point, and one I wasn't aware of. I 💯 agree that this can be worked around with a custom Django middleware (this is the solution we have deployed to production today) However I'm raising this PR as it seems... daft to force everyone to:
So I feel it would be better if: a. This was supported "out of the box" by the AWS SDK So... final update to this PR where I take these comments into account incoming. |
|
@carolabadeer see update -> which matches my desire to "avoid people getting the wrong default behaviour" and allows smart users to configure for either ALL or NONE as appropriate. |
| if self.in_lambda_ctx: | ||
| segment = xray_recorder.begin_subsegment(name) | ||
| segment = xray_recorder.begin_subsegment(name, namespace="remote") |
There was a problem hiding this comment.
Can you please clarify the reason for setting namespace="remote" here? As far as I know, setting a subsegment's namespace as remote manually makes it show on the service map and should not affect the ability to search annotations.
There was a problem hiding this comment.
Helpfully directly from the Xray team here:
change the namespace of the subsegments from "local" to "remote" and to then use the service filter. When the namespace is set to "remote", Xray will infer a node base which can be searched up with the service filter.
So this enables service filter use in this case too
There was a problem hiding this comment.
I referred back to the team and confirmed that we do index all annotations in a trace, which includes annotations added on subsegments. In this case, it doesn't seem necessary to set namespace="remote" in order to be able to search annotations, so we can merge this PR once that adjustment is made!
There was a problem hiding this comment.
Thank you! Updated with removing this. Will plan to remove our patches once this is released, and looking forward to nobody else hitting this issue!
This enables searching by URL values for implementations of Django in Lambda working around the restriction that http.url can't be searched on subsegments in the X-Ray Console.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.