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

Lambda instrumentation implementation #32

Merged
merged 11 commits into from
Oct 10, 2019

Conversation

bonybrown
Copy link
Contributor

Issue #20

Description of changes:

  • Adds aws-xray-sdk/lambda.rb to be used in lambda functions to initialise and configure XRay.recorder.
  • Adds subclasses of existing Default Context, Emitter and Streamer classes to customise these object's behaviours in the lambda context
  • Adds a facade Segment object that the lambda uses as the root segment for all subsegments recorded in the lambda.

This work is based off the implementation of the python xray-sdk for lambda.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. ✔️

@bonybrown
Copy link
Contributor Author

Also, created this project that deploys a Lambda function that demonstrates the forked gem.
https://github.com/bonybrown/aws-lambda-ruby-xray-demo

lib/aws-xray-sdk/lambda/facade_segment.rb Show resolved Hide resolved
lib/aws-xray-sdk/lambda/lambda_context.rb Show resolved Hide resolved
lib/aws-xray-sdk/lambda/lambda_emitter.rb Outdated Show resolved Hide resolved
@bonybrown
Copy link
Contributor Author

All requested changes made and confirmed to be working in https://github.com/bonybrown/aws-lambda-ruby-xray-demo.

@srprash
Copy link
Contributor

srprash commented Sep 9, 2019

Hi @bonybrown ,
Great work. I really appreciate your contribution. I tested the code on my end and also checked out the demo app you put together.
I am experience a little issue with both. When I disable the Active tracing from the lambda function console, I still see an empty trace generated in the X-Ray console. Please see the following images.

Screen Shot 2019-09-09 at 2 59 07 PM

And this is the trace data:
Screen Shot 2019-09-09 at 2 59 30 PM

Are you seeing this empty trace as well?

@bonybrown
Copy link
Contributor Author

Just tried, and yes, I'm seeing the same behaviour. Interesting that with Active Tracing disabled in the console, the REPORT output still shows an XRay trace id:

END RequestId: 7238dcbb-84d7-401f-943c-fff00b1c10da
REPORT RequestId: 7238dcbb-84d7-401f-943c-fff00b1c10da	Duration: 259.67 ms	Billed Duration: 300 ms	Memory Size: 128 MB	Max Memory Used: 55 MB	
XRAY TraceId: 1-5d76e4fe-4481878076dd858f6f8c4547	SegmentId:

I'll investigate and get back to you.

ref_counter and subsegment_size mutators on FacadeSegment must be implemented
so that adding sub-subsegments works.
@bonybrown
Copy link
Contributor Author

Hi @srprash - problem identified and fixed. In doing so, identified another problem, now also fixed.
Tested with Active Tracing turned on AND off.
No longer creating empty traces when the lambda is not sampled.

@bonybrown
Copy link
Contributor Author

Hi @srprash, any update on the progress of this PR? I believe all the issues raised have been addressed.

@srprash
Copy link
Contributor

srprash commented Sep 23, 2019

Hi @bonybrown ,
Sorry for the delay. We are talking to the internal team to see if there's a fix for the localhost entity that we are seeing in the lambda function so that we can avoid the workaround in the PR. Once, it is sorted, we can get this PR merged.

Thanks,
Prashant

@@ -16,12 +16,18 @@ def new(*options)
module HTTPInstanceInterceptor
include XRay::Facets::Helper

# Pattern matches the path of requests from the AWS Lambda Ruby Runtime.
# We do not want to capture these
LAMBDA_RUNTIME_PATH_PATTERN = /\A\/[0-9-]{10}\/runtime\/invocation\//.freeze
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of using regex for pattern matching, I think we can fetch the value of AWS_LAMBDA_RUNTIME_API environment variable and check if the req.path begins with this value. I just came across this env variable in lambda runtime and we can avoid using regex matching altogether.

Copy link
Contributor

Choose a reason for hiding this comment

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

req.path would only give the partial path that does not include the host information. Instead req.uri gives the complete url http://127.0.0.1:9001/...... Doing a start_with? check on req.uri would work better. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to test against the AWS_LAMBDA_RUNTIME_API environment variable.
Tested in sample app and seems to be working fine.

Copy link
Contributor

@srprash srprash left a comment

Choose a reason for hiding this comment

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

I keep getting "ERROR -- : can not find the current context." while running the lambda function. Are you getting this too?

lib/aws-xray-sdk/facets/net_http.rb Outdated Show resolved Hide resolved
lib/aws-xray-sdk/lambda/lambda_recorder.rb Show resolved Hide resolved
@bonybrown
Copy link
Contributor Author

Sorry, seems that my demo project was using the prior version of aws-xray-sdk-ruby - note to self: it's important to completely remove the vendor directory when changing versions of gems from git.

Have fixed and added tests of begin_segment and begin_subsegment to the demo project.

 # Test begin_segment - should report warning, but not create segment
XRay.recorder.begin_segment 'my_segment'
puts "in segment - begin_segment call should have logged a warning"
# Test end_segment - should report warning
XRay.recorder.end_segment

# Test begin_subsegment, should result in subsegment created
XRay.recorder.begin_subsegment 'sub-segment'
puts "in subsegment"
dynamo_client.list_tables
uri = URI 'https://ip-ranges.amazonaws.com/ip-ranges.json'
http_connection.request(uri).body
XRay.recorder.end_subsegment

Using aws-xray-sdk 0.11.2 from https://github.com/bonybrown/aws-xray-sdk-ruby.git (at lambda-instrumentation@d2d7223) and updated demo project I now get this output:

START RequestId: 7301fb2e-249d-45e7-a468-51167a136a33 Version: $LATEST
W, [2019-10-07T23:45:57.171513 #8]  WARN -- : Cannot create segments inside Lambda function. Returning current segment.
in segment - begin_segment call should have logged a warning
W, [2019-10-07T23:45:57.186110 #8]  WARN -- : Cannot end segment inside Lambda function. Ignored.
in subsegment
END RequestId: 7301fb2e-249d-45e7-a468-51167a136a33
REPORT RequestId: 7301fb2e-249d-45e7-a468-51167a136a33	Duration: 996.39 ms	Billed Duration: 1000 ms	Memory Size: 128 MB	Max Memory Used: 64 MB	Init Duration: 927.05 ms	
XRAY TraceId: 1-5d9bce34-db783d334fd2d50cabd9e15a	SegmentId: 0bc8f498226155ce	Sampled: true	

I don't get ERROR -- : can not find the current context. at all, and attempting to begin a segment results in a warning log message (the existing facade segment is returned) as expected.

@bonybrown
Copy link
Contributor Author

This is the x-ray trace for the invocation logged above ( TraceId: 1-5d9bce34-db783d334fd2d50cabd9e15a )
image

@srprash
Copy link
Contributor

srprash commented Oct 8, 2019

Looks good and ready to be merged!
Thanks for your contribution.

@srprash srprash merged commit 46e97ad into aws:master Oct 10, 2019
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

Successfully merging this pull request may close these issues.

2 participants