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

_X_AMZN_TRACE_ID is not set #85

Closed
PeterAdams-A opened this issue May 25, 2020 · 10 comments
Closed

_X_AMZN_TRACE_ID is not set #85

PeterAdams-A opened this issue May 25, 2020 · 10 comments
Labels
discussion for things that need discussion

Comments

@PeterAdams-A
Copy link

The AWS guide - https://docs.aws.amazon.com/lambda/latest/dg/runtimes-custom.html - says a custom runtime should do the following

Propagate the tracing header – Get the X-Ray tracing header from the Lambda-Runtime-Trace-Id header in the API response. Set the _X_AMZN_TRACE_ID environment variable locally with the same value. The X-Ray SDK uses this value to connect trace data between services.

The _X_AMZN_TRACE_ID environment variable is not currently set.

@fabianfett
Copy link
Member

Hi @PeterAdams-A,

thanks for bringing this up. We currently expose the xray trace header on the Context.traceId property.

As far as I am aware there is currently none Swift xray-sdk which might use the env variable. There is however a Google Summer of Code project that will move tracing with Swift forward. I’m pretty sure that we will update the AWSLambdaRuntime to use this tracing approach as soon the work on it has been finished.

I see a number of difficulties with exposing the traceId as an env variable. Especially if we are creating new trace segments (which maybe running concurrently), how do we know the parent?

Would you mind explaining in which instances you would rather get the traceId from the environment vs. having this set on the context object?

Adding @ktoso to the discussion.

@ktoso
Copy link
Contributor

ktoso commented May 25, 2020

Hi folks,
just realized you don't know each other (yet :-)).

Yup, keeping our eyes on this and traces as we'll work with Moritz (gsoc) on the context propagation.

The env variable thing sounds a bit weird... We'll definitely want to use context propagation anyway since otherwise it'd get lost between futures or (swift-)async-http-client calls etc. So just the env var seems a bit unrelated... but let's give it a deeper look when we have some PoC context.

Great that the lambda lib already exposes the trace id as context.traceId, we'll need to work on composition there using the context lib but that's exactly what's planned anyway :)

@ktoso
Copy link
Contributor

ktoso commented May 25, 2020

FYI, the context propagation (tracing underpinnings) work will be happening over here https://github.com/slashmo/gsoc-swift-tracing

@PeterAdams-A
Copy link
Author

Hi :-)

I actually don't have a use case for the environment variable - I was just pointing out that the AWS docs imply that you should set it.

In fact the reason I looked for this environment variable in the code was because I was interested to see what you'd done with it as it always seemed like such a horrible thing to do when I'd look at this in the past - it effectively introduces a really nasty process global.

I'm not sure why AWS seem to like this, I did wonder if they had an external monitoring process which queried the process environment or something. I think @tomerd was going to ask them.

@tomerd
Copy link
Contributor

tomerd commented May 28, 2020

cc @bmoffatt who may have the answer on why the runtime impl needs to set it

@tomerd tomerd added the discussion for things that need discussion label May 28, 2020
@bmoffatt
Copy link
Contributor

It was a design decision that predates the Runtime API. There's no need to set or mutate the variable (aside from cosmetic consistency with existing docs and other runtimes)

@ktoso
Copy link
Contributor

ktoso commented May 29, 2020

Cool, thanks for confirming!

@Andrea-Scuderi
Copy link
Contributor

It's required to trace the Lambda with X-ray according to the documentation:
https://docs.aws.amazon.com/lambda/latest/dg/runtimes-custom.html

Propagate the tracing header – Get the X-Ray tracing header from the Lambda-Runtime-Trace-Id header in the API response. Set the _X_AMZN_TRACE_ID environment variable locally with the same value. The X-Ray SDK uses this value to connect trace data between services

I added it in my runtime to be able to evaluate the performances: https://github.com/swift-sprinter/aws-lambda-swift-sprinter-core/blob/master/Sources/LambdaSwiftSprinter/Sprinter.swift

The user can enable/disable it from the console so I think it should be set.

Screenshot 2020-05-29 at 21 57 53

It's important to evaluate the performances:

Screenshot 2020-05-29 at 22 00 42

@fabianfett
Copy link
Member

Hi @Andrea-Scuderi, this runtime exposes the tracing header on the Context object for further context/trace propagation and in the logger as metadata for better log consolidation. If you enable tracing right now you will get an xray output just like you've shown above.

Since @bmoffatt has told us that we don't need to expose the header as an env variable I would argue to close this issue since it's original purpose doesn't need to be fulfilled. @tomerd wdyt?

@tomerd
Copy link
Contributor

tomerd commented May 29, 2020

@bmoffatt is def the authority on this, so yes closing sgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion for things that need discussion
Projects
None yet
Development

No branches or pull requests

6 participants