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

Moving from Jaeger exporter to OTLP exporter usage by default #7597

Merged
merged 6 commits into from
Dec 4, 2022

Conversation

ppatierno
Copy link
Member

This PR fixes #7592 moving from the Jaeger exporter to the OLTP exporter usage by default when using OpenTelemetry.

This PR cannot be merged yet and is in draft fot two main reason:

  • it needs a new bridge 0.23.0 having OTLP as well, otherwise it will deploy KC, MM and MM2 with OTLP but old bridge 0.22.3 with Jaeger exporter
  • the tracing system tests have to be updated by using Jaeger 1.35 as minimum version because it's the first version providing an OTLP endpoint.

@ppatierno
Copy link
Member Author

Current status:

  • for Kafka Connect and Mirror Maker system tests I already updated them to use the OTLP endpoint as collector on the Jaeger backend side
  • for Bridge related test I haven't change the endpoint yet because we need a new bridge release with the OTLP exporter in (so I cannot test this PR yet)
  • regarding the used Kafka clients (see KafkaTracingClients class), it seems they use the image coming from the test-clients project ... can you confirm this @im-konge ? These test clients have to be update by using the opentelemetry-exporter-otlp and we should coordinate. If we change it in the test-clients to allow this PR to work (in terms of system tests), then the regression tests could fail for other PRs still using Kafka images with old Jaeger exporter (until this one will be merged and the migration to OTLP exporter finished). Wdyt @scholzj ?

@im-konge
Copy link
Member

@ppatierno yes, the KafkaTracingClients are using the test-clients images, but it has fixed version, so I can add PR for the opentelemetry-exporter-otlp and after that do a micro release.
After I will release it, you can use it in this PR
So we will not break anything with PR to the main branch of test-clients

@im-konge
Copy link
Member

So, after the test-clients will be released (the micro version 0.4.1), here are all the updates needed in the code to make it work
https://gist.github.com/im-konge/1c512e5592b21e7cea533eab678ebbb7
Maybe the OTEL_TRACES_EXPORTER will not be needed, if bridge is able to have otlp as default without need of setting that env variable

@ppatierno ppatierno mentioned this pull request Nov 28, 2022
1 task
Signed-off-by: Paolo Patierno <ppatierno@live.com>
Signed-off-by: Paolo Patierno <ppatierno@live.com>
…rter

Signed-off-by: Paolo Patierno <ppatierno@live.com>
Maker system tests

Signed-off-by: Paolo Patierno <ppatierno@live.com>
Signed-off-by: Paolo Patierno <ppatierno@live.com>
@ppatierno ppatierno marked this pull request as ready for review December 2, 2022 14:53
@ppatierno ppatierno requested a review from a team December 2, 2022 14:53
@ppatierno
Copy link
Member Author

@im-konge I have updated the PR using your suggestions. Could you have another pass on the PR please?

Signed-off-by: Paolo Patierno <ppatierno@live.com>
@ppatierno ppatierno requested a review from im-konge December 2, 2022 15:56
@ppatierno
Copy link
Member Author

@strimzi-ci run tests test_only profile=regression testcase=OpenTelemetryST,OpenTracingST

@strimzi-ci
Copy link

Systemtests Failed (no tests results are present)

@im-konge
Copy link
Member

im-konge commented Dec 2, 2022

/azp run regression

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ppatierno
Copy link
Member Author

Regression includes the tracing STs and they are green, so we are good to merge. Not sure why the Jenkins one failed. Wdyt @scholzj @im-konge ?

@im-konge
Copy link
Member

im-konge commented Dec 4, 2022

I don't think that it's related to PR, Jenkins was down or something like that in the time you triggered it

I will check it

@im-konge
Copy link
Member

im-konge commented Dec 4, 2022

@ppatierno it failed just because we didn't have Java 17 on our containers, that should be OK now.

@ppatierno
Copy link
Member Author

@im-konge thanks for checking! My point was that it's now useless running the Jenkins pipeline because you already ran the regression(s) and they include the tracing STs. Everything is green. So I can merge it.

@ppatierno ppatierno merged commit f190b0d into strimzi:main Dec 4, 2022
@ppatierno ppatierno deleted the move-otlp-exporter branch December 4, 2022 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrating from OpenTelemetry Jaeger exporter to OTLP exporter
4 participants