search

aws / aws-xray-sdk-go

AWS X-Ray SDK for the Go programming language.
Apache License 2.0
275 stars 118 forks source link

fix: #425 - protobuf upgrade #469

Closed MLCarey321 closed 1 week ago

MLCarey321 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 4 weeks ago

@wangzlei I updated the tests last week -- can you run the workflows again?

MLCarey321 commented 3 weeks ago

@wangzlei Can you run the workflows again?

MLCarey321 commented 3 weeks ago

@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 commented 2 weeks ago

@wangzlei What are the next steps here?