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

WIP/Feedback: X-Ray daemon & service integrations. #737

Closed
wants to merge 7 commits into from

Conversation

Tanbouz
Copy link

@Tanbouz Tanbouz commented Oct 29, 2018

Issue: [Feature Request] Add support for X-Ray on SAM Local #217

This does not replicate AWS X-Ray service itself. It attempts to:

The X-Ray segments will still be emitted to your AWS account if the AWS creds allow so. Better use a non-production AWS account!

Description of changes:

  • Optionally run (--xray -x only in start-api) an X-Ray daemon service in a docker container alongside other invoked Lambdas.
  • Enable Lambda's X-Ray integration by emitting segments created by real Lambdas. AWS::Lambda and AWS::Lambda::Function.
  • Make usable X-Ray environment variables inside the Lambda runtime (Working _X_AMZN_TRACE_ID and let segment = new AWSXRay.getSegment() for example)
  • 🚨 Blocking - PR to upstream Lambda docker container to allow _X_AMZN_TRACE_ID to be available inside Lambda in all runtimes. Fix only applied to nodejs8.10 for testing Tanbouz/docker-lambda@4328937
  • ⚠️ Important - Respect SAM template Lambda's Tracing setting.
  • ⚠️ Important - Improve handling of a failed X-Ray daemon service.
  • ⚠️ Important - Maybe allow exposed port to be reconfigured by user to avoid port conflicts.
  • ⚠️ Important - Attempt to make generated X-Ray Integration segments match as closely as possible.
  • 🍰 Additional feature - X-Ray API Gateway integration.
  • 🔮 Future - Tests awaiting feedback and finalizing API/features.
  • 🔮 Future - Respect SAM template API Gateway's X-Ray setting Add x-ray enable flag to API gateway serverless-application-model#581

Sample
X-Ray segments submitted from SAM CLI. Interceptor was my Lambda function name. The crossed out subsegment was added inside my Lambda function code.
X-Ray segments submitted from SAM CLI

Other issues:

  • X-Ray daemon seems to buffer segments and didn't work at least from my testing if started and stopped with a Lambda function. Must be a long running background process. That is why only start-api for now.

Tests:

  • Lint pass
  • Tests were mostly fixed locally - awaiting feedback.
  • Fails due to code coverage

Requesting feedback.
😞 Had a hard time figuring where to place the code within the project structure too, feedback welcome

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

@Tanbouz Tanbouz changed the title X-Ray daemon & service intergrations. [WIP/Feedback] X-Ray daemon & service integrations. Oct 29, 2018
@Tanbouz Tanbouz changed the title [WIP/Feedback] X-Ray daemon & service integrations. WIP/Feedback: X-Ray daemon & service integrations. Oct 29, 2018
@sanathkr
Copy link
Contributor

Hey! Thanks for sending this pull request :) Definitely an interesting feature to surface to customers. We will review the code shortly and help you with open questions.

@Tanbouz Tanbouz changed the base branch from develop to master December 1, 2018 15:34
@Tanbouz Tanbouz changed the base branch from master to develop December 1, 2018 15:35
@jfuss
Copy link
Contributor

jfuss commented Dec 3, 2018

@Tanbouz Awesome! I am going to start looking through this PR now.

We appreciate the time you have put into this! Would you be willing to create a design doc for this. It will help capture the change and give everyone a way to digest what is going on. We are trying to do this for all new features.

@jfuss
Copy link
Contributor

jfuss commented Dec 3, 2018

@Tanbouz I am having a hard time understanding the use-case for supporting AWS X-Ray locally. X-Ray is great for profiling your production (even pre-production) Lambda Functions but why would you want to profile locally? What is the benefit of doing this locally vs doing this on the environment directly (aka AWS Lambda)?

I think a design doc for this would go a long way. Good questions to cover:

  • Who would this be valuable to?
  • How would this be valuable to customers?
  • What use-cases require AWS X-Ray locally?
  • How does this work with Lambda to Lambda calls locally?

@thesriram noticed that AWS X-Ray only supports tracing for applications that are written in Node.js, Java, and .NET (https://aws.amazon.com/xray/features/). This leaves out many other languages that we currently support.

@Tanbouz
Copy link
Author

Tanbouz commented Dec 4, 2018

First of all, sorry had an incident updating my branch from upstream.

Design doc 👍, just to quickly answer these for now...

Who would this be valuable to? @jfuss

I'm curious too about other use cases in #217

How would this be valuable to customers?

My own use case in bold:

Many instrumentation scenarios require only configuration changes. For example, you can instrument all incoming HTTP requests and downstream calls to AWS services that your Java application makes. To do this, you add the X-Ray SDK for Java's filter to your servlet configuration, and take the AWS SDK for Java Instrumentor submodule as a build dependency. For advanced instrumentation, you can modify your application code to customize and annotate the data that the SDK sends to X-Ray. -- AWS X-Ray Use Cases and Requirements

(-1 for PR) X-Ray is already awesome and supports patching HTTP clients or SQL connectors which I guess doesn't require much work from a developer or require SAM supporting X-Ray locally during development.

(+1 for PR) My intent was not to profile locally but develop custom X-Ray subsegments that wrap critical parts of my code that I would like to profile later when it goes into production.

How does this work with Lambda to Lambda calls locally?

If we decided to move forward, it is something I would like to explore along with supporting it across SAM CLI (single lambda invokes..etc), not only in start-api.

Supported Languages: @thesriram

Not sure why that document mentions only 3 languages. Conflicts with:
https://docs.aws.amazon.com/xray/latest/devguide/xray-usage.html#xray-usage-languages

I'm also testing against a live Python Lambda function with active tracing enabled.

@jfuss
Copy link
Contributor

jfuss commented Jul 26, 2019

The team appreciates the time and effort you put into this @Tanbouz. At this time, we want to find a better way to support X-Ray related use cases. The team understands this is a customer pain point in developing and running locally but we want to ensure we can maintain the solution (keeping this updated as X-Ray updates, performance impacts with needing to start multiple containers, etc).

Please add further thoughts, ideas, use-cases in the tracking issue: #217

@jfuss jfuss closed this Jul 26, 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.

3 participants