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.

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

Jump to bottom

Updated Documentation For Change in Behaviour of Reading X-Ray Tracing Info From Lambda #27

Merged
merged 10 commits into from
May 25, 2023
Merged

Updated Documentation For Change in Behaviour of Reading X-Ray Tracing Info From Lambda #27

merged 10 commits into from
May 25, 2023

Conversation

jdoherty
Copy link
Contributor

@jdoherty jdoherty commented May 15, 2023 •
edited
Loading

Related issues #

open-telemetry/opentelemetry-java-instrumentation#8368

open-telemetry/opentelemetry-java-instrumentation#7579

Related OTEP(s) #

@jsuereth asked I open this change here - rather than over in the documentation repo (open-telemetry/opentelemetry-specification#3502)

@tylerbenson , @carlosalberto, @Oberon00 FYI I know you previously reviewed the other PR so this is now open here as requested by Josh

@jdoherty jdoherty requested review from a team May 15, 2023 14:45
@jdoherty jdoherty requested a review from a team May 16, 2023 08:25
jsuereth
jsuereth approved these changes May 16, 2023
View reviewed changes
Copy link
Contributor

@jsuereth jsuereth left a comment

Choose a reason for hiding this comment

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

LGTM pending @tylerbenson approval as well.

tylerbenson
tylerbenson approved these changes May 16, 2023
View reviewed changes
Copy link
Member

@tylerbenson tylerbenson left a comment

Choose a reason for hiding this comment

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

Minor wording suggestions but otherwise looks good to me.

specification/trace/semantic_conventions/instrumentation/aws-lambda.md Outdated Show resolved Hide resolved
jdoherty and others added 2 commits May 17, 2023 12:07
trask
trask reviewed May 17, 2023
View reviewed changes
Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

Is there any AWS Lambda documentation that mentions the use (and name) of the system property?

@smirnoal
Copy link

Is there any AWS Lambda documentation that mentions the use (and name) of the system property?

not yet, but I'm working with AWS docs team on this to be documented.
As for now it is used in the source codes for Lambda Java Runtime and AWS SDK (both open source)

@trask
Copy link
Member

trask commented May 18, 2023

As for now it is used in the source codes for Lambda Java Runtime and AWS SDK (both open source)

can you add links to these usages? that's good enough for me, thx

@smirnoal
Copy link

As for now it is used in the source codes for Lambda Java Runtime and AWS SDK (both open source)

can you add links to these usages? that's good enough for me, thx

here you are, Trask:

AWS x-ray SDK:
https://github.com/aws/aws-xray-sdk-java/blob/711e48cfd54400e728f99bec3bb3baf417276b46/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/contexts/LambdaSegmentContext.java#L40

AWS Lambda Java runtime:
https://github.com/aws/aws-lambda-java-libs/blob/main/aws-lambda-java-runtime-interface-client/src/main/java/com/amazonaws/services/lambda/runtime/api/client/AWSLambda.java#L62

@jdoherty jdoherty mentioned this pull request May 18, 2023
Merged
@jdoherty
Copy link
Contributor Author

Hi @jsuereth now we have merged open-telemetry/opentelemetry-java-instrumentation#8368
and everyone on this chain has approved this one - can you ask a fellow approver / maintainer to take a look please?

@jsuereth
Copy link
Contributor

@jdoherty Looks good, can you add a changelog?

@jsuereth
Copy link
Contributor

Can you fix the lint errors? I'll merge as soon as I see green.

@jdoherty
Copy link
Contributor Author

Can you fix the lint errors? I'll merge as soon as I see green.

Thank you :-). Darn whitespace. I've now removed this and rebased the other changes from mainline. Running the lint locally shows no errors for me now.

Do you need to kick off the workflow? I see my push does not trigger it.

@jsuereth
Copy link
Contributor

This still needs one full approver, until we promote some FAAS folks to CODEOWNERS.

@jdoherty @Oberon00
Update specification/trace/semantic_conventions/instrumentation/aws-l…
77f1aaa 
…ambda.md

Co-authored-by: Christian Neumüller <christian+github@neumueller.me>
@jsuereth jsuereth merged commit eacb63d into open-telemetry:main May 25, 2023
@joaopgrassi joaopgrassi mentioned this pull request Jun 19, 2023
Merged
@smirnoal
Copy link

smirnoal commented Dec 5, 2023

Is there any AWS Lambda documentation that mentions the use (and name) of the system property?

not yet, but I'm working with AWS docs team on this to be documented. As for now it is used in the source codes for Lambda Java Runtime and AWS SDK (both open source)

docs were updated: https://docs.aws.amazon.com/lambda/latest/dg/configuration-envvars.html#configuration-envvars-runtime

@tylerbenson tylerbenson mentioned this pull request Dec 13, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smirnoal smirnoal smirnoal approved these changes

@tylerbenson tylerbenson tylerbenson approved these changes

@Oberon00 Oberon00 Oberon00 approved these changes

@trask trask trask approved these changes

@jsuereth jsuereth jsuereth approved these changes

Assignees

@jsuereth jsuereth

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jdoherty @smirnoal @trask @jsuereth @tylerbenson @Oberon00