Skip to content

Conversation

MLCarey321
Copy link

@MLCarey321 MLCarey321 commented Jun 6, 2024

Issue #, if available: #425

Description of changes:
Upgraded protobuf from github.com/golang/protobuf v1.5.3 to google.golang.org/protobuf v1.33.0. Required updates to some other dependencies, mainly github.com/grpc-ecosystem/go-grpc-middleware v1.3.0 to github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.1.0. Because the naming conventions are more explicit in v2, I also had to update some of the types used in grpc_test.go.

I'll note that the tests fail, but with the same failure as they did when I ran the tests before making any changes, namely:

--- FAIL: TestBadRoundTripDial (0.08s)
    client_test.go:392: 
        	Error Trace:	/aws-xray-sdk-go/xray/client_test.go:392
        	Error:      	An error is expected but got nil.
        	Test:       	TestBadRoundTripDial
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x30 pc=0x105782690]

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

@MLCarey321 MLCarey321 requested a review from a team as a code owner June 6, 2024 20:40
@MLCarey321
Copy link
Author

Not sure who can/should review, but I see that @wangzlei is the only user that both commented on #425 that I am able to tag. :)

@MLCarey321
Copy link
Author

I opened issue #471 for the failing test -- I'm getting this same error when I run make test from master with no changes.

@MLCarey321

This comment was marked as outdated.

@MLCarey321

This comment was marked as outdated.

@MLCarey321
Copy link
Author

@wangzlei I don't have access to a Window's machine for testing and I'm not sure why the test is failing -- it's in capture_test.go:74, which shouldn't have been affected by my changes...

@MLCarey321
Copy link
Author

@wangzlei What are the next steps here?

@wangzlei wangzlei merged commit 38e7a20 into aws:master Jul 8, 2024
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