Ensure that all our handlers extend ExtHandler

[dmlloyd]

Fixes #44540 (in theory)

[quarkus-bot]

/cc @brunobat (opentelemetry), @radcortez (opentelemetry)

[gsmet]

@dmlloyd when you say in theory did you test this particular example?

[dmlloyd]

I tested it obliquely, but I couldn't test it exactly (yet?) due to local configuration problems.

[gastaldi]

@MaxFichtelmann can you build this PR locally and confirm that it fixes your issue before we merge it?

[brunobat]

It makes sense to me even if it doesn't fix the bug.

[MaxFichtelmann]

@MaxFichtelmann can you build this PR locally and confirm that it fixes your issue before we merge it?

Sure, I'll build and Test when I am at the PC again.

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 8fe3c76.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

[MaxFichtelmann]

hm, still seeing that behaviour in quarkus dev - actually when running that same project without dev, the issue does not appear, neither with 3.14.3 nor 3.16.3 - so seems to be a devmode-only issue.

[gastaldi]

I agree with @brunobat. Even if it doesn't fix the issue, I think this is a nice improvement to have

[dmlloyd]

I also opened jboss-logging/jboss-logmanager#493 which should address what we're seeing on the stack trace, once that is merged & the component is updated.

[gastaldi]

I'd recommend to wait until jboss-logging/jboss-logmanager#493 is merged and released and updated in this PR before merging

[dmlloyd]

I don't think waiting is necessary, because the two changes are not interdependent.

[gsmet]

@dmlloyd sorry if it's just noise but how will the new patch in JBoss Logmanager interact with this specific piece:

7e6c529#diff-3027189c5073a902729f033232338719972cd18a3c64018f24b99dd78e1fea6aR441-R447

where we add parameters to something that used to be NO_FORMAT.

[gastaldi]

@dmlloyd I know, but the second change will fix the bug, right?

[dmlloyd]

It might :-)

[gastaldi]

Loving the confidence behind this patch :D

[dmlloyd]

@dmlloyd sorry if it's just noise but how will the new patch in JBoss Logmanager interact with this specific piece:

7e6c529#diff-3027189c5073a902729f033232338719972cd18a3c64018f24b99dd78e1fea6aR441-R447

where we add parameters to something that used to be NO_FORMAT.

The interaction should be fine. The handler for NO_FORMAT here will change it to use message format, with two arguments.

[gsmet]

The interaction should be fine. The handler for NO_FORMAT here will change it to use message format, with two arguments.

And it won't fail if we end up with My message { that used to work in NO_FORMAT but won't work with message format? Or maybe you don't resolve the parameters recursively?

[dmlloyd]

The interaction should be fine. The handler for NO_FORMAT here will change it to use message format, with two arguments.

And it won't fail if we end up with My message { that used to work in NO_FORMAT but won't work with message format? Or maybe you don't resolve the parameters recursively?

The original message is passed as a parameter to a new message, so no further interpolation can take place.

[gsmet]

OK, perfect, thanks for the clarification. I'm still a bit puzzled the OP says it's failing only in dev mode but I wonder if he actually used the newly compiled version in dev mode, I will have to check that.

[MaxFichtelmann]

OK, perfect, thanks for the clarification. I'm still a bit puzzled the OP says it's failing only in dev mode but I wonder if he actually used the newly compiled version in dev mode, I will have to check that.

I am optimistic that I used the updated version, but this is the first time I built quarkus myself, so...