8000 Add annotations by millarm · Pull Request #348 · aws/aws-xray-sdk-python · GitHub
[go: up one dir, main page]
Skip to content

Add annotations #348

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Aug 8, 2022
Merged

Add annotations #348

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2a70787
Add annotations
millarm Jul 25, 2022
6a2fa07
Merge branch 'aws:master' into master
millarm Aug 2, 2022
3ddb808
Update based on PR discussions
millarm Aug 2, 2022
9d5ee2f
Merge branch 'master' of https://github.com/millarm/aws-xray-sdk-python
millarm Aug 2, 2022
21cc7fc
Remove namespace='remote' as it's not necessary for indexing
millarm Aug 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,13 @@ If `AUTO_PATCH_PARENT_SEGMENT_NAME` is also specified, then a segment parent wil
with the supplied name, wrapping the automatic patching so that it captures any dangling
subsegments created on the import patching.

### Django in Lambda
X-Ray can't search on http annotations in subsegments. To enable searching the middleware adds the http values as annotations
This allows searching in the X-Ray console like so
```
annotation.url BEGINSWITH "https://your.url.com/here"
```

### Add Flask middleware

```python
Expand Down
21 changes: 19 additions & 2 deletions aws_xray_sdk/ext/django/middleware.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,12 @@ def __call__(self, request):
recorder=xray_recorder,
sampling_req=sampling_req,
)

http_as_annotations = False
if self.in_lambda_ctx:
segment = xray_recorder.begin_subsegment(name)
segment = xray_recorder.begin_subsegment(name, namespace="remote")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor
@carolabadeer carolabadeer Aug 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

# X-Ray can't search/filter subsegments on URL but it can search annotations
# So for lambda to be able to filter by annotation we add these as annotations
http_as_annotations = True
else:
segment = xray_recorder.begin_segment(
name=name,
Expand All @@ -64,23 +67,37 @@ def __call__(self, request):
segment.save_origin_trace_header(xray_header)
segment.put_http_meta(http.URL, request.build_absolute_uri())
segment.put_http_meta(http.METHOD, request.method)
if http_as_annotations:
segment.put_annotation(http.URL, request.build_absolute_uri())
segment.put_annotation(http.METHOD, request.method)

if meta.get(USER_AGENT_KEY):
segment.put_http_meta(http.USER_AGENT, meta.get(USER_AGENT_KEY))
if http_as_annotations:
segment.put_annotation(http.USER_AGENT, meta.get(USER_AGENT_KEY))
if meta.get(X_FORWARDED_KEY):
# X_FORWARDED_FOR may come from untrusted source so we
# need to set the flag to true as additional information
segment.put_http_meta(http.CLIENT_IP, meta.get(X_FORWARDED_KEY))
segment.put_http_meta(http.X_FORWARDED_FOR, True)
if http_as_annotations:
segment.put_annotation(http.CLIENT_IP, meta.get(X_FORWARDED_KEY))
segment.put_annotation(http.X_FORWARDED_FOR, True)
elif meta.get(REMOTE_ADDR_KEY):
segment.put_http_meta(http.CLIENT_IP, meta.get(REMOTE_ADDR_KEY))
if http_as_annotations:
segment.put_annotation(http.CLIENT_IP, meta.get(REMOTE_ADDR_KEY))

response = self.get_response(request)
segment.put_http_meta(http.STATUS, response.status_code)
if http_as_annotations:
segment.put_annotation(http.STATUS, response.status_code)

if response.has_header(CONTENT_LENGTH_KEY):
length = int(response[CONTENT_LENGTH_KEY])
segment.put_http_meta(http.CONTENT_LENGTH, length)
if http_as_annotations:
segment.put_annotation(http.CONTENT_LENGTH, length)
response[http.XRAY_HEADER] = prepare_response_header(xray_header, segment)

if self.in_lambda_ctx:
Expand Down
0