Upgraded OTel to 0.20.0 #2807
Upgraded OTel to 0.20.0 #2807
Conversation
Signed-off-by: Joe Elliott <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #2807 +/- ##
=======================================
Coverage 95.95% 95.95%
=======================================
Files 218 218
Lines 9620 9624 +4
=======================================
+ Hits 9231 9235 +4
Misses 322 322
Partials 67 67
Continue to review full report at Codecov.
|
| @@ -80,7 +82,7 @@ func (b *ConnBuilder) CreateConnection(logger *zap.Logger, mFactory metrics.Fact | |||
| return nil, errors.New("at least one collector hostPort address is required when resolver is not available") | |||
| } | |||
| if len(b.CollectorHostPorts) > 1 { | |||
| r, _ := manual.GenerateAndRegisterManualResolver() | |||
| r := generateAndRegisterManualResolver() | |||
There was a problem hiding this comment.
is it preemptive change needed to upgrade grpc later?
There was a problem hiding this comment.
It's due to the otel grpc dependency upgrading here:
https://github.com/jaegertracing/jaeger/pull/2807/files#diff-c941bfc88e74f7f559df1ab09fdfeb7c228f0dfce84ef863555812222a943e25R1469
and since the otel components use this functionality it needs to be up to date with 1.35.0.
There was a problem hiding this comment.
sorry, that link doesn't point me to anywhere. I just meant that there's no grpc upgrade in go.mod in this PR.
There was a problem hiding this comment.
the grpc upgrade is actually due to the otel upgrade. was trying to link to the go.sum changes under ./cmd/opentelemetry, but it does not appear to work.
0.20.0 otel collector requires grpc 1.35.0. since the components under ./cmd/opentelemetry reference Jaeger core, it also has to be up to date with 1.35.0 even though it is not explicitly using it in its own go.mod.
There was a problem hiding this comment.
I see. Have you tried bumping grpc to the same ver in the main go.mod? I just tried it and it seems to work, and I think it's better to keep the two in sync
$ go get google.golang.org/[email protected]
go: downloading google.golang.org/grpc v1.35.0
go: downloading google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013
$ make build-collector
(no errors)
There was a problem hiding this comment.
Have you tried bumping grpc to the same ver in the main go.mod?
Isn't this what dependabot did in another PR?
There was a problem hiding this comment.
google.golang.org/grpc is currently at 1.29.1.
attempting an upgrade now to 1.35.0, but getting some failed tests. i will see if i can't iron these out today.
There was a problem hiding this comment.
Might be worth checking this: #2771
Signed-off-by: Joe Elliott <[email protected]>
mergify
bot
dismissed
yurishkuro’s stale review
Pull request has been modified.
Signed-off-by: Joe Elliott <[email protected]>
|
@joe-elliott should we close this? |
|
Yup, I think this no longer makes sense. |
Which problem is this PR solving?
Short description of the changes
generateAndRegisterManualResolverfrom the "google.golang.org/grpc/resolver/manual" package. This appears to be a test function we are using to init the resolver? Unsure why this was used.queued_retryprocessor as it no longer exists.Statusobject can no longer be nil. Adjusted code accordingly.