CAMEL-23719: camel-micrometer - when ProducerTemplate sends to a SEDA endpoint, Exchange.getFromRouteId() returns null#23864
Conversation
davsclaus
left a comment
There was a problem hiding this comment.
Thank you for the PR and the detailed reproducer — the bug report is valid. When ProducerTemplate sends to a SEDA endpoint, getFromRouteId() returns null, causing getTags() to produce a tag set without routeId. Other exchanges produce tags with routeId. Since Micrometer Prometheus now requires consistent tag keys across meters with the same name, this causes an IllegalArgumentException.
However, the fix approach has some issues:
1. Semantic mismatch — fromRouteId is the route the exchange originated from, not the route it's going to. The lookup finds the destination route by matching endpoint URIs, which is a different concept.
2. Ambiguity — Multiple routes can consume from the same endpoint (e.g., multiple SEDA consumers). findFirst() would pick one arbitrarily.
3. Performance — Iterating all routes on every exchange event is expensive in large contexts with many routes.
4. Incomplete — Many cases won't find a match anyway (dynamic endpoints, timer endpoints, etc.), so the tag inconsistency would still occur for those.
The simpler correct fix is to always include the routeId tag, using an empty string when null. This ensures consistent tag keys across all meters. This is the same approach used in CAMEL-21661 (commit 25b9ce69ae2b) which fixed the same class of problem for getInflightExchangesTags. The entire getTags() method becomes:
String routeId = event.getExchange().getFromRouteId();
return Tags.of(
CAMEL_CONTEXT_TAG, event.getExchange().getContext().getName(),
KIND, KIND_EXCHANGE,
EVENT_TYPE_TAG, event.getClass().getSimpleName(),
ROUTE_ID_TAG, routeId != null ? routeId : "",
ENDPOINT_NAME, uri,
FAILED_TAG, Boolean.toString(event.getExchange().isFailed()));Also, the project requires a JIRA ticket for bug fixes, and commit messages should follow the CAMEL-XXXX: Brief description format.
Please feel free to open a JIRA issue and rework with the simpler approach. Happy to help if you have questions!
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
c4ad39e to
89c99d3
Compare
89c99d3 to
4b56199
Compare
Hello @squakez, makes sense to me, I just pushed the requested changes. Can you review and let me know if something else is needed? |
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet
left a comment
There was a problem hiding this comment.
Clean fix for the camel-micrometer exchange event notifier. The changes correctly:
-
Ensure
routeIdtag is always present in timer metrics — using an empty string whenfromRouteIdis null instead of omitting the tag entirely. This prevents Micrometer from creating timers with inconsistent tag key sets (which causesIllegalArgumentExceptionwhen mixing route-originated and ProducerTemplate-originated exchanges). -
Add
getOrCreateTimer()helper to consolidate timer lookup/creation logic and avoid re-registering timers. -
Include a well-structured test (
MicrometerExchangeEventNotifierProducerTemplateTest) that validates consistent tag keys across both route and ProducerTemplate exchanges.
The approach of using an empty string for the missing routeId tag is the right call — it maintains tag cardinality consistency while making it clear the exchange didn't originate from a route.
Fully automatic review from Claude Code
…when RouteId is set Ensure Prometheus Event Notification happens when a RouteId is configured in the destination Route of a message sent by a ProducerTemplate.
…rometheus focused test Ensure MicrometerExchangeEventNotifier registers Exchange Event Timers with Tag Keys so Prometheus export works when Route triggered and ProducerTemplate Exchanges are mixed. - Reuse existing timers via getOrCreateTimer() for sent and completed events; - Add ProducerTemplate test with Prometheus scrape validation; - Add micrometer-registry-prometheus test dependency; - Document routeId tag behavior change in the 4.21 upgrade guide. Generated-by: Cursor Agent on behalf of @fernandobalieiro.
4b56199 to
e592da9
Compare
|
🧪 CI tested the following changed modules:
✅ POM dependency changes: targeted tests included Modules affected by dependency changes (2)
All tested modules (12 modules)
|
|
Thanks for the help |
No problem at all, thank you for the quick review and merge. |
Description
The goal of this PR is to ensure that the Prometheus Event Notification happens when a RouteId is configured in the destination Route of a message sent through a ProducerTemplate.
Current Scenario
When a project is configured to use
quarkus-micrometer-registry-prometheusand a message is sent to aSEDAendpoint throughProducerTemplate, the following warning appears in the logs:The issue seems to be caused by recent changes in the Micrometer framework, as reported in the Quarkus 3.35 Release Notes:
https://github.com/quarkusio/quarkus/wiki/Migration-Guide-3.35#micrometer-prometheus-registry
A reproducer can be found in this repository. The message will appear by either running the project in Quarkus dev mode and invoking http://localhost:8080/hello endpoint or by executing the
ProducerRouteITtest.Or execute the integration tests directly:
The warning message containing the stack trace should appear in the logs.
Expected Scenario
The event should be properly registered and no warning message should be visible in the logs.
Consideration
I'm aware that this is maybe not the best approach, as the solution requires iterating over all routes to retrieve the target routeId, so I'm open to suggestions.
Target
mainbranch)Tracking
Apache Camel coding standards and style
I checked that each commit in the pull request has a meaningful subject line and body.
I have run
mvn clean install -DskipTestslocally from root folder and I have committed all auto-generated changes.