Bump OTEL version to latest #2355
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This has been unblocked by: open-telemetry/opentelemetry-collector#1574 I have been working through the Jaeger Otel Collector integration and would like to give this PR a review when its ready. |
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
pavolloffay
force-pushed
the
bump-otel22
branch
from
a7a61e5
to
cbde9f0
Compare
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
|
@joe-elliott would you like to review? I have updated the PR |
|
We might have to update proto |
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
|
The issue is solved by Weird is that the badger test fails on |
|
Seems like a problem with |
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
|
pinning the gogo and protobuf verisons that are currently used in Jaeger worked. We should upgrade proto the grpc deps in a separate PR. |
joe-elliott
approved these changes
Aug 20, 2020
Makefile
Outdated
| @@ -165,7 +165,8 @@ lint-gosec: | |||
| .PHONY: lint-staticcheck | |||
| lint-staticcheck: | |||
| @cat /dev/null > $(LINT_LOG) | |||
| time staticcheck ./... \ | |||
| # TODO remove SA1019 once https://github.com/golang/protobuf/issues/1077 is resolved | |||
| time staticcheck -checks=-SA1019 ./... \ | |||
There was a problem hiding this comment.
Instead of ignoring SA1019 universally does it make more sense to add the offending files to the list below?
| grep -v \
-e model/model.pb.go \
-e model/prototest/model_test.pb.go \
-e thrift-gen/ \
The other two files (cmd/collector/app/zipkin/http_handler_test.go, model/ids_test.go) are not autogenerated and could have nolint directives added. Although to keep things in one spot, might be better to add all three here with a note to remove once golang/protobuf#1077 is resolved.
There was a problem hiding this comment.
This does not seem to be needed after I removed proto version bump. I will remove it for now.
We will run into this when upgrading to the newer proto.
| factories.Receivers["jaeger"] = &jaegerreceiver.Factory{ | ||
| Wrapped: jaegerRec, | ||
| Wrapped: otelJaegerReceiver.NewFactory(), |
There was a problem hiding this comment.
Previously the same factory was passed into both jaegerreceiver.Factory shims. I don't see any harm in doing it this way, and actually prefer this stylistically. Unsure if there are consequences I'm unaware of.
There was a problem hiding this comment.
They removed factory types and instead they use just factory method from the helper package.
| @@ -15,29 +15,31 @@ | |||
| package resourceprocessor | |||
There was a problem hiding this comment.
Clever reuse of the otel resource processor. As far as I can tell this is being configured to support agent tags?
There was a problem hiding this comment.
To inject tags passed as flags or env property. The OTEL collector does not support flags or env vars at the moment.
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
Codecov Report
@@ Coverage Diff @@
## master #2355 +/- ##
=======================================
Coverage 95.60% 95.60%
=======================================
Files 206 206
Lines 10549 10549
=======================================
Hits 10085 10085
+ Misses 396 395 -1
- Partials 68 69 +1
Continue to review full report at Codecov.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Blocked by open-telemetry/opentelemetry-collector#1411