Micrometer Observation component #9389
Conversation
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 If necessary Apache Camel Committers may access logs and test results in the job summaries! |
github-actions
bot
added
catalog
components
core-build-and-dependencies
docs
camel-3
labels
...vation/src/test/java/org/apache/camel/observation/CamelMicrometerObservationTestSupport.java
Outdated
Show resolved
Hide resolved
Components tested:
|
Components tested:
|
This is not a problem, we could do that later
This could be done with a subsequent PR
On this, I'm not really sure about it: if we goes this way it's important to document and notes the difference, otherwise users might think the result will be the same with Micrometer and Opentelemetry and it won't be the case
|
|
Please rebase on 3.x, so we are good. Waiting for @orpiske and @davsclaus reviews. |
|
Thanks for the big work, there is a little merge conflict, I wonder if you could resolve that and push update to this PR so it can be merged cleanly. Also we would need this to be ported to Camel v4 (main) but that is JDK17 and JakartaEE; which I assume micrometer also works with, but just wanted to mention the difference to Camel v3. |
Added low cardinality tags, changed the implementation to be as close as possible to the OTel one. Left the handlers as examples of how to make the behaviour equal to the OTel one.
marcingrzejszczak
force-pushed
the
micrometer-observation
branch
from
116d179
to
e227a34
Compare
|
I fixed the conflicts and did some additional changes
Below you have a screenshot of a trace view for a very simple route
With the Micrometer Observation you also get the following metric. I also changed the behaviour so that it is close to the OpenTelemetry one but not the same - I will document the differences plus I've left the code in the tests that can be used to have the same behaviour. So the question to you is - do you find such information valuable (traces and metrics) in its current form if you were to troubleshoot any issues within a Camel based system? I can try a more complex scenario and we can discuss what metrics / tags can be more useful in your opinion. We can also play around with the names if you think e.g. name of the component should be there in a span name. As for Jakarta - Micrometer is working fine with JDK17. |
|
So I've been playing around with this and I'm really surprised with how it's actually working currently with OpenTelemetry. Let's assume that I have the following routes from("direct://foo")
.log("hello")
.to("direct://bar")
.to("http://example.org");
from("direct://bar")
.log("hello")
.to("direct://micrometer")
.to("http://example.org");
from("direct://micrometer")
.setHeader(MicrometerConstants.HEADER_METRIC_NAME, constant("new.name"))
.setHeader(MicrometerConstants.HEADER_METRIC_TAGS, constant(Tags.of("dynamic-key", "dynamic-value")))
.to("micrometer:counter:name.not.used?tags=key=value")
.to("direct://baz");
from("direct://baz")
.log("hello")
.to("bean:com.example.cameldemo.MyBean")
.to("exec:wc?args=--words /usr/share/dict/words")
.process(exchange -> {
// Use the Camel Exec String type converter to convert the ExecResult to String
// In this case, the stdout is considered as output
String wordCountOutput = exchange.getIn().getBody(String.class);
// do something with the word count
System.out.println(wordCountOutput);
})
.to("file:///tmp/camel-outputdir?flatten=true")
.to("http://example.org");I would assume that for the whole flow (e.g. from
Is this really the intent of how this should actually work? I would assume that there should be 1 trace for the whole processing because otherwise this doesn't make a lot of sense (at least to me). |
|
As far as I remember it was not really easy to have a full trace, in the end we were able to have one trace for each route. This means from..to, when you pass to another from..to, like for example This will be another trace. I don't remember the reason, but I think instrumenting it to take into account all the routes for a single trace was too hard to implement or too invasive. I might remember wrongly nevertheless |
This will actually not be a trace because |
This looks wrong then. Can you open an issue? |
|
Thanks |
Components tested:
|
|
IMO the graph should like this (please correct me if I'm wrong). Actually it's still missing calls to example.org, file or exec from("direct://foo")
.log("hello")
.to("direct://bar")
.to("http://example.org");
from("direct://bar")
.log("hello")
.to("direct://micrometer")
.to("http://example.org");
from("direct://micrometer")
.setHeader(MicrometerConstants.HEADER_METRIC_NAME, constant("new.name"))
.setHeader(MicrometerConstants.HEADER_METRIC_TAGS, constant(Tags.of("dynamic-key", "dynamic-value")))
.to("micrometer:counter:name.not.used?tags=key=value")
.to("direct://baz");
from("direct://baz")
.log("hello")
.to("bean:com.example.cameldemo.MyBean")
.to("exec:wc?args=--words /usr/share/dict/words")
.process(exchange -> {
// Use the Camel Exec String type converter to convert the ExecResult to String
// In this case, the stdout is considered as output
String wordCountOutput = exchange.getIn().getBody(String.class);
// do something with the word count
System.out.println(wordCountOutput);
})
.to("file:///tmp/camel-outputdir?flatten=true")
.to("http://example.org");
We have a red graph beacuse I'm explicitly throwing an exception in a controller in my application. I've achieved this graph by removing the instanceof check here (https://github.com/apache/camel/blob/camel-3.x/components/camel-tracing/src/main/java/org/apache/camel/tracing/Tracer.java#L296) but obviously I might be missing sth. I'm not committing that change, but if you confirm my assumptions I can file an issue and point to a potential solution. EDIT: I'm still wondering why I don't see a span for the external call to |
|
Actually everything is fine after doing the before mentioned change. I didn't have the file or exec only because I was throwing an exception in the bean component and the exchange processing was stopped 🤦 So how do we proceed? I mean should I commit the change with the exclusion or not? |
|
The latest observation screenshot looks really great, with all the span details. @marcingrzejszczak are you referring to the |
Yes, I'm referring to that. So I made it work by changing the method from this private boolean shouldExclude(SpanDecorator sd, Exchange exchange, Endpoint endpoint) {
return sd instanceof AbstractInternalSpanDecorator || !sd.newSpan()
|| isExcluded(exchange, endpoint);
}to this private boolean shouldExclude(SpanDecorator sd, Exchange exchange, Endpoint endpoint) {
return !sd.newSpan()
|| isExcluded(exchange, endpoint);
} |
|
Ah okay, so |
|
So without my change afair you'll see spans but not for direct components and you'll not see a single trace for the whole flow of a message but you'll see multiple different traces. That's because spans are no longer maintaining parent child relationship. |
|
Okay I think we can accept that code change. Are all the unit test passing with this PR in both camel-tracing and camel-micrometer ? |
If I do the change to the |
|
Maybe we could mark them as disabled and get back to them once we have the fix in place. |
|
It's up to you - I don't have a problem with adding back the code from the |
|
If you time to do that, it would be nice, we are probably in the same situation on main, 3.x and 3.20.x, so we should fix the shouldExclude method on LTS branch too (3.20.x). To me it's +1. Thanks a lot for your time. |
|
OK so I'm changing this PR as ready for review |
|
Hopefully I fixed checkstyle issues and also I've created the new issue here https://issues.apache.org/jira/browse/CAMEL-19124 |
|
I have no idea why https://github.com/apache/camel/actions/runs/4314828399/jobs/7528671102 is failing - I ran the build locally and it's fine |
|
Yeah. the github action fails for some of these checkstyle on 3.x but its a false error. Thanks for the hard work on this. |
|
Can you work on the SB starter for the 3.x branch, so we have all together. Then afterwards we can possible cherry pick to main branch. If not then we appreciate if you would then work on PRs for main so we have this in v4 as well. |
|
merging this for 3.x so its included in Camel 3.21 onwards. |
|
Also I wonder if you would contribute an example such as for camel-spring-boot-examples. And you are welcome to include screenshots in the example as it helps users to quickly see this awesome feature |
|
Screenshots is also welcome in docs. However they may be harder to do, and the img may need to go in docs root folder where we have images for other pages such as EIPs |
|
Sure I can work on the starter. Of course the screenshots should be done after we fix the issue with shouldExclude method because the trace view will be broken. Also, I can work on the examples. |
|
Spring Boot configuration apache/camel-spring-boot#784 |
A list of questions and TODOs related to this component that we must resolve before merging
Discuss:
Tracerabstraction from Camel has a notion of only high cardinality tags, we are missing low cardinality tags for metrics (that means that metrics will not have any tags at this point)Tracerin thetracingcomponent acceptable?TODO:
Recrated from #9377