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

Bump to 3.10.x Quarkus.io and align libraries #3872

Merged
merged 4 commits into from
Jan 14, 2025

Conversation

matzew
Copy link
Contributor

@matzew matzew commented May 2, 2024

Fixes #

Proposed Changes

Release Note


Docs

@knative-prow knative-prow bot requested review from creydr and pierDipi May 2, 2024 11:38
@knative-prow knative-prow bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 2, 2024
@matzew
Copy link
Contributor Author

matzew commented May 2, 2024

Let's see how this goes ...
/hold

@knative-prow knative-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 2, 2024
Copy link

codecov bot commented May 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 45.44%. Comparing base (bcfacd5) to head (db70907).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3872   +/-   ##
=======================================
  Coverage   45.44%   45.44%           
=======================================
  Files         270      270           
  Lines       19893    19893           
=======================================
  Hits         9041     9041           
  Misses      10133    10133           
  Partials      719      719           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@matzew
Copy link
Contributor Author

matzew commented May 2, 2024

/test reconciler-tests

@matzew
Copy link
Contributor Author

matzew commented May 3, 2024

/retest

@pierDipi pierDipi changed the title Bump to new 3.10.x Quarkus.io [WIP] Bump to new 3.10.x Quarkus.io May 8, 2024
@knative-prow knative-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 8, 2024
@matzew
Copy link
Contributor Author

matzew commented May 8, 2024

/retest

@knative-prow-robot knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 14, 2024
@knative-prow-robot knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 16, 2024
@knative-prow-robot knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 8, 2024
@matzew
Copy link
Contributor Author

matzew commented Jul 17, 2024

/test unit-tests

@knative-prow-robot knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 17, 2024
@matzew matzew force-pushed the quarkus_3.10 branch 2 times, most recently from bd94e45 to 3d381ff Compare July 23, 2024 15:46
Copy link
Contributor

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Reopen with /reopen. Mark as fresh by adding the
comment /remove-lifecycle stale.

@github-actions github-actions bot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 22, 2024
@knative-prow-robot knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 2, 2024
@knative-prow-robot knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 7, 2025
@@ -57,7 +57,8 @@ public LoomKafkaProducer(Vertx v, Producer<K, V> producer) {
final var ctxInt = ((ContextInternal) v.getOrCreateContext()).unwrap();
if (ctxInt.tracer() != null) {
this.tracer =
new ProducerTracer(ctxInt.tracer(), TracingPolicy.PROPAGATE, "" /* TODO add bootrstrap servers */);
new ProducerTracer(this.vertx.tracer(), TracingPolicy.PROPAGATE, "" /* TODO add bootrstrap servers */);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tsegismont Even changing this, still is causing the ReceiverVerticleTracingTest unit test to fail

matzew and others added 4 commits January 14, 2025 10:42
Signed-off-by: Matthias Wessendorf <[email protected]>
Signed-off-by: Matthias Wessendorf <[email protected]>
In Vert.x 4.5.1, there was a fix to use the OTel default context storage when the Vert.x context storage provider is not invoked on a Vert.x thread.
See eclipse-vertx/vertx-tracing#72

Since LoomKafkaProducer invokes the tracer on a virtual thread (or a worker thread), the sending task must be wrapped with the OTel current context.
Otherwise, tracing data will be lost at this point.

OTel provides an ExecutorService that does just that, so this commit refactors the producer to use an executor service instead of a queue plus manual polling.

Important: note that with this implementation, a virtual thread is created for each record, which is different from sending them one after the other with a single thread.
@matzew matzew changed the title [WIP] Bump to new 3.10.x Quarkus.io Bump to new 3.10.x Quarkus.io Jan 14, 2025
@knative-prow knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 14, 2025
@matzew matzew changed the title Bump to new 3.10.x Quarkus.io Bump to 3.10.x Quarkus.io and align libraries Jan 14, 2025
@pierDipi
Copy link
Member

/test reconciler-tests-namespaced-broker

@matzew
Copy link
Contributor Author

matzew commented Jan 14, 2025

/unhold

@knative-prow knative-prow bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 14, 2025
Copy link
Member

@pierDipi pierDipi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jan 14, 2025
Copy link

knative-prow bot commented Jan 14, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: matzew, pierDipi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@matzew
Copy link
Contributor Author

matzew commented Jan 14, 2025 via email

@knative-prow knative-prow bot merged commit 0b4e723 into knative-extensions:main Jan 14, 2025
33 checks passed
@tsegismont
Copy link
Contributor

Glad I was able to contribute here 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/data-plane lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants