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

AWS X-Ray config module for Lambda instrumentation #984

Merged
merged 59 commits into from
Oct 26, 2021

Conversation

bhautikpip
Copy link
Contributor

Draft as waiting on merge of #882 . Currently based off #882 source so will require a quick rebase before being opened as full PR.

Changes not included in #882 are found in commit eef81a9 and later

bhautikpip and others added 30 commits February 4, 2021 16:21
@bhautikpip
Copy link
Contributor Author

@Aneurysm9 @MrAlias @dashpole @punya Feel free to take a look on this PR.

Copy link
Contributor

@willarmiros willarmiros left a comment

Choose a reason for hiding this comment

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

Thanks, lgtm with just a few small comments!

CHANGELOG.md Outdated Show resolved Hide resolved
}
```

Now configure the instrumentation with the provided options to export traces to AWS X-Ray via [the OpenTelemetry Collector](https://github.com/open-telemetry/opentelemetry-collector) running as a Lambda Extension. Instructions for running the OTel Collector as a Lambda Extension can be found in the [AWS OpenTelemetry Documentation](https://aws-otel.github.io/docs/getting-started/lambda).
Copy link
Contributor

Choose a reason for hiding this comment

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

Made a suggestion.

@bhautikpip
Copy link
Contributor Author

@Aneurysm9 addressed your comments!

@willarmiros
Copy link
Contributor

@MrAlias Could you take a quick look at this PR if you have a chance?

@Aneurysm9
Copy link
Member

This test needs to be fixed:

=== RUN   TestWrapEndToEnd
    xrayconfig_test.go:137: 
        	Error Trace:	xrayconfig_test.go:137
        	            				xrayconfig_test.go:176
        	Error:      	Not equal: 
        	            	expected: &v1.InstrumentationLibrary{state:impl.MessageState{NoUnkeyedLiterals:pragma.NoUnkeyedLiterals{}, DoNotCompare:pragma.DoNotCompare{}, DoNotCopy:pragma.DoNotCopy{}, atomicMessageInfo:(*impl.MessageInfo)(nil)}, sizeCache:0, unknownFields:[]uint8(nil), Name:"go.opentelemetry.io/contrib/instrumentation/github.com/aws/aws-lambda-go/otellambda", Version:"semver:0.24.0"}
        	            	actual  : &v1.InstrumentationLibrary{state:impl.MessageState{NoUnkeyedLiterals:pragma.NoUnkeyedLiterals{}, DoNotCompare:pragma.DoNotCompare{}, DoNotCopy:pragma.DoNotCopy{}, atomicMessageInfo:(*impl.MessageInfo)(nil)}, sizeCache:0, unknownFields:[]uint8(nil), Name:"go.opentelemetry.io/contrib/instrumentation/github.com/aws/aws-lambda-go/otellambda", Version:"semver:0.25.0"}
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -13,3 +13,3 @@
        	            	  Name: (string) (len=83) "go.opentelemetry.io/contrib/instrumentation/github.com/aws/aws-lambda-go/otellambda",
        	            	- Version: (string) (len=13) "semver:0.24.0"
        	            	+ Version: (string) (len=13) "semver:0.25.0"
        	            	 })
        	Test:       	TestWrapEndToEnd
--- FAIL: TestWrapEndToEnd (0.17s)

Copy link
Member

@Aneurysm9 Aneurysm9 left a comment

Choose a reason for hiding this comment

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

Once the test failure is fixed, this looks good to me.

@jmacd
Copy link
Contributor

jmacd commented Oct 26, 2021

(assuming the above suggestions are resolved!)

@Aneurysm9 Aneurysm9 merged commit 32e2f1c into open-telemetry:main Oct 26, 2021
@alolita
Copy link
Member

alolita commented Oct 27, 2021

@dashpole @Aneurysm9 can you please re-review?

@alolita
Copy link
Member

alolita commented Oct 27, 2021

Thanks! @jmacd @dashpole @Aneurysm9

@MrAlias MrAlias mentioned this pull request Oct 27, 2021
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.

None yet

8 participants