-
Notifications
You must be signed in to change notification settings - Fork 717
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
JMS instrumentation #584
Comments
One thing I discussed with @marcingrzejszczak about this issue, is that it is virulent: at the point we start propagating different field names, we have the concern of whether the other side can read it or not. For this reason, I'm very in-favor of JMS specific notions of this. A lot of messaging providers are unaffected by this problem and can safely pass headers from http directly into messaging headers. Even if it feels tempting to make a "messaging" header name, we have to keep in mind that many are already using header names with hyphens. If we start propagating underscores universally, we introduce complexity to existing non-JMS consumers. Luckily, JMS header mapping is a problem sites using it already know. It is mechanical for people to rewrite headers from hyphens to something else, even if annoying. It is above why I'm biased towards containing this mapping problem as close as practical to JMS and not spreading the infection beyond it, potentially into innocent frameworks with no such constraints. If we choose to do the infectious route, it will imply folks who aren't using JMS will have to start adding what might feel like spaghetti code. /me ends commentary |
to fix this longer, we've rallied to have the trace headers not have hyphens in the w3c spec https://github.com/w3c/distributed-tracing/tree/master/trace_context |
openzipkin/b3-propagation#21 potentially solves this |
Any planning for jms instrumentation in brave ? Interested via spring sleuth usage. +1 for this. |
read my mind.. was going to start on this today. brave 5.3 will be
mostly messaging related
|
this needs to merge prior to working on JMS #763 |
folks who want JMS to happen should emoji or reply on this comment. Until the format is sorted, we are blocked openzipkin/b3-propagation#21 (comment) |
Can folks here offer how they are using JMS at the moment? via spring-jms, camel, MDBs, or something else? I want to make sure the README is relevant |
spring-jms talking to weblogic 12, Oracle Service Bus |
I'll make a separate module for spring-jms. Can anyone review this from a user-friendliness and/or implementation perspective? #764 |
Some are using spring-jms to ActiveMQ. And some services using ActiveMQ component with JMS listener with Camel Routes. Sample configuration:
|
Spring jms via Spring boot activeMQ starter.
JMSTemplate for message publication and @JmsListener for message
consumption.
Le lun. 27 août 2018 à 11:13, Jorg Heymans <notifications@github.com> a
écrit :
… spring-jms talking to weblogic 12, Oracle Service Bus
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#584 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGR-iFkCYk4tl8AFy2kfZwhmYCf2-aFRks5uU7gqgaJpZM4Rlr4_>
.
|
updated #764 with JMS 2.0 apis (entrypoint of TracingJMSContext). please give feedback on this, if you use |
On our snapshot repo (http://oss.jfrog.org/oss-snapshot-local) 5.3.0-SNAPSHOT includes brave-instrumentation-jms |
Added #778 to track spring-jms as right now message listeners that are handled by spring aren't invoked in the context of a parent producer span |
Do you have any example regarding, how to integrate brave with Apache ActiveMQ ? |
Im still facing this problem with Spring, Camel and Jms. Is there already any fix available for this problem ? |
Yes. Just Upgrade to last version. |
Oops wrong post, I am using Jaeger not Zipkin. I guess the update wont work for me. |
There's an indirect requirement for JMS instrumentation through spring-cloud-sleuth. Besides the multiple versions of JMS apis, there's an important concern with propagation due to naming constraints which exist in JMS, but not necessarily in the underlying transport.
JMS requires that message header names follow java naming conventions. Notably this excludes hyphens and dots. For example, in Camel, there's a naming policy to map to and from these constraints.
For example, many simply downcase B3 headers to
x-b3-traceid
for use in non-http transports (which might not be case insensitive). In JMS, this wouldn't work, even if rabbitmq is used underneath which has no such constraint. Ex, if using camel JMS, this header would map intox_HYPHEN_b3_HYPHEN_traceid
.In some cases, JMS headers are replaced based on constants or other patterns, like globally replacing hyphens with underscores. Interestingly opentracing decided on a different pattern
_$dash$_
, though they have no pattern for dots: For example, if you were using the OT library and camel on the other, you'd getx_HYPHEN_b3_HYPHEN_traceid
on one side andx_$dash$_b3_$dash$_traceid
on the other, mutually unreadable.The universal implication of using JMS is that it implies global coordination of these naming patterns, or there will be propagation incompatibility (ex traces will restart). This will also break anything else propagated that isn't a trace ID. This type of problem has already been noticed in early users of message tracing.
Importantly, even if we were to have static replacements for B3, we don't know if users will introduce propagated headers with dots or hyphens in them. In other words, I don't think a solution to this problem can be constant in nature, though constants could help.
Challenge here is to introduce support for JMS propagation, but without assuming we are using B3 format, and ideally inheriting a naming convention as opposed to introducing another dimension of naming convention complexity for others. Coming up with a sane default might be particularly tricky.
Calling for advice from @davsclaus @garyrussell @Xylus @jkowall @christian-schwarzbauer @CodingFabian (others feel free to contribute too!)
See also discussion on w3c trace context https://github.com/w3c/distributed-tracing/issues/35
The text was updated successfully, but these errors were encountered: