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

Add observability support for JMS #30335

Closed
vpavic opened this issue Apr 13, 2023 · 20 comments
Closed

Add observability support for JMS #30335

vpavic opened this issue Apr 13, 2023 · 20 comments
Assignees
Labels
in: messaging Issues in messaging modules (jms, messaging) theme: observability An issue related to observability and tracing type: enhancement A general enhancement
Milestone

Comments

@vpavic
Copy link
Contributor

vpavic commented Apr 13, 2023

See #30094 for some background.

I'm primarily interested in the following:

  • adding trace information to the JMS message on the producer side (when using JmsOperations)
  • resolving trace information from the JMS message on the consumer side (when using either JmsOperations directly or @JmsListener)

The goal is to have spans from both the producer and the consumer side as a part of the same trace.

I think that something like this is possible when using Spring Integration (see the relevant docs), but it would be nice if core Spring JMS support also had support for observability.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 13, 2023
@bclozel bclozel added in: messaging Issues in messaging modules (jms, messaging) type: enhancement A general enhancement theme: observability An issue related to observability and tracing and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Apr 14, 2023
@bclozel bclozel added this to the 6.1.x milestone Apr 14, 2023
@bclozel bclozel self-assigned this Apr 14, 2023
@simasch
Copy link

simasch commented May 27, 2023

This was working with Sleuth, and after switching to micrometer tracing this is no longer working.
Any plan to re-add this support?

@snicoll
Copy link
Member

snicoll commented May 27, 2023

Any plan to re-add this support?

@simasch I am not sure I understand your question. You are commenting on the very issue doing that, scheduled for 6.1.x...

@simasch
Copy link

simasch commented May 27, 2023

@snicoll Oh I'm sorry. I didn't notice the milestone! Probably I shouldn't work on weekends 😅

@Saljack
Copy link

Saljack commented Jul 20, 2023

Sorry for bothering but I have to ask. Is there any chance that it will be in Spring 6.1? I am asking because most of the missing pieces of observability were resolved and this is still untouched. Is there any blocker in JMS that blocks it?

@bclozel
Copy link
Member

bclozel commented Jul 20, 2023

@Saljack no worries. I'm currently working on it and this is targeted for the next milestone.

Most of the instrumentation is not Spring specific so I'm preparing a PR for Micrometer to generally instrument Jakarta JMS Session instances.

I'll report back here as soon as I've got news.

@bclozel
Copy link
Member

bclozel commented Aug 2, 2023

Here is the PR for the Jakarta JMS instrumentation in Micrometer core: micrometer-metrics/micrometer#4007
I'm also done with the support in Spring Framework for both JmsTemplate and @JmsListener and will push the changes as soon as the PR is merged in Micrometer.

bclozel added a commit to bclozel/spring-framework that referenced this issue Aug 2, 2023
This commit adds observability support for Jakarta JMS support in
spring-jms support. This feature leverages the `JmsInstrumentation`
infrastructure in `io.micrometer:micrometer-core` library.

This instruments the `JmsTemplate` and the `@JmsListener` support to
record observations:

* "jms.message.publish" when the `JmsTemplate` sends a message
* "jms.message.process" when a message is processed by a `@JmsListener`
  annotated method

The observation `Convention` and `Context` implementations are shipped
with "micrometer-core".

Closes spring-projectsgh-30335
bclozel added a commit to bclozel/spring-framework that referenced this issue Aug 2, 2023
This commit adds observability support for Jakarta JMS support in
spring-jms support. This feature leverages the `JmsInstrumentation`
infrastructure in `io.micrometer:micrometer-core` library.

This instruments the `JmsTemplate` and the `@JmsListener` support to
record observations:

* "jms.message.publish" when the `JmsTemplate` sends a message
* "jms.message.process" when a message is processed by a `@JmsListener`
  annotated method

The observation `Convention` and `Context` implementations are shipped
with "micrometer-core".

Closes spring-projectsgh-30335
@snicoll
Copy link
Member

snicoll commented Aug 14, 2023

This broke Spring Boot's build.

AbstractMessageListenerContainer has a hard dependency on io.micrometer.core.instrument.binder.jms.JmsProcessObservationConvention which requires micrometer-core:

See this build scan

@snicoll snicoll reopened this Aug 14, 2023
izeye added a commit to izeye/spring-framework that referenced this issue Sep 5, 2023
bclozel pushed a commit that referenced this issue Sep 5, 2023
@nightswimmings
Copy link

@bclozel Should his beta feature include Span/TraceId propagation code? I did a small test with latest milestone and producer did not put any tracing header auomatically nor receiver decode it, at least via JmsTemplate calls. I don't see any mention to trace/span in neither your MR nor micrometer one (metrics-side but not tracing), so Ill ask just in case if this remains unsupported

@bclozel
Copy link
Member

bclozel commented Sep 14, 2023

Is your current setup producing metrics with the JmsTemplate?
The JmsPublishObservationContext should propagate the trace context information through message properties.

@mbadziong
Copy link

I face the same problem. Have prepared two minimal examples - one with old sleuth setup, new with micrometer. I have no tracing even with SP 3.1.4 (which has all micrometer updates etc).
Link to minimalistic examples: https://github.com/mbadziong/micrometer-broken-tracing

@Saljack
Copy link

Saljack commented Sep 14, 2023

@mbadziong You have to use Spring Boot 3.2 RC.

@nightswimmings
Copy link

@Saljack You mean v3.2.0-M2, right?
That's the one I am testing, if so.

Anyway! My issue is fixed. My bad, I presumed the autoconfiguration worked in a postprocessor/customizer fashion, but once I explictly set the ObservationRegistry on my locally exposed JmsTemplates or JmsListenerContainerFactorys everything is supersmooth.

I'll check in an E2E across services, if I see something I will open a bug. Thanks @bclozel !

@nightswimmings
Copy link

@bclozel Just a very nitpicking suggestion, but adding the ObservationRegistry setter to JmsDefaultListenerContainerSpec would be nice!

@bclozel
Copy link
Member

bclozel commented Sep 14, 2023

@nightswimmings the JmsDefaultListenerContainerSpec lives in the Spring Integration project, could you create an issue there to ask for this enhancement please?

As for the lack of auto-configuration in Spring Boot, that's a good point. Spring Boot currently auto-configures a JmsTemplate bean but does not offer any customizer to augment it. I've created spring-projects/spring-boot#37388 to address that.

@tw-atroehrsm
Copy link

tw-atroehrsm commented Oct 10, 2023

FYI. Micrometer Jms Implementation seems to be faulty, it is not escaping the Property Names according to defined standard.
micrometer-metrics/micrometer#4202

@engkimbs
Copy link

engkimbs commented Nov 23, 2023

@mbadziong is there any working sample project what you've talked about jms with micrometer configuration? I'm new to JMS and micrometer with spring boot 3. It will be helpful if you give a working sample project.

@WrRaThY
Copy link

WrRaThY commented Jan 8, 2024

@nightswimmings

can you provide an example of what you did?
I'm running boot 3.2.1 and there is still no autoconfiguration for the JMS micrometer

@bclozel
Copy link
Member

bclozel commented Jan 8, 2024

@WrRaThY do you have the micrometer-jakarta9 dependency on your classpath as explained in the reference docs?

@Saljack
Copy link

Saljack commented Jan 9, 2024

@WrRaThY
You need a few things. At least these dependencies:

    <!-- Observability / Tracing is implemented in Actuator. Check that you really use it  -->
    <dependency>
      <groupId>org.springframework.boot</groupId>
      <artifactId>spring-boot-starter-actuator</artifactId>
    </dependency>

    <!-- Tracing library. It enrich clients (JmsTemplate) and processes incoming requests (JmsListener). You can replace it with OTEL -->
    <dependency>
      <groupId>io.micrometer</groupId>
      <artifactId>micrometer-tracing-bridge-brave</artifactId>
    </dependency>

    <!-- It is for reporting to Zipkin, you can omit it if you do not want to report tracing or replace it with a different reporter -->
    <dependency>
      <groupId>io.zipkin.reporter2</groupId>
      <artifactId>zipkin-reporter-brave</artifactId>
    </dependency>

Then you need to check if your JmsTemplate is autoconfigured or if you create it somewhere else. If you create it yourself then you have to set there an ObservationRegistry. (There was a change in Spring Boot 3.2.1 because in Spring Boot 3.2.0 it worked without adding it because there was a BeanPostProcessor which enhanced all JmsTemplate instances.)

jmsTemplate.setObservationRegistry(observationRegistry);

The same thing you have to do for JmsListenerContainerFactory. If it is not autoconfigured then there is also a method setObservationRegistry or you can use DefaultJmsListenerContainerFactoryConfigurer (this is a better option in my opinion).

I had to also disable consuming B3 headers because if a message does not contain any tracing header then it (also with disabling B3 Multi) throws an IllegalArgumentException because of trying to extract X-B3-Sampled and it is an invalid JMS header. For more details see: micrometer-metrics/micrometer#4202 (comment)

management.tracing.propagation.consume=W3C

So I would start with debugging if your JmsTemplate has configured any ObservationRegistry and then if your messages contain tracing headers/properties (search for traceparent in properties). And then in listeners.

I hope I did not forget anything important.

I would also check your JMS library because for example we use Azure ServiceBus JMS which uses QPid and there is their autoconfiguration in the Azure library which overrides the original from Spring Boot.

@WrRaThY
Copy link

WrRaThY commented Jan 9, 2024

Thank you for such a wholesome answer @Saljack
I had the deps in, but those bean registration tips are very interesting. I'm sure that they will become very useful once I start upgrading other apps. Much appreciated.

When it comes to this particular issue: I modified the minimal example project that was shared here and with boot 3.2.1 everything works out of the box. Communication between 2.7.18 and 3.2.1 works as well. No bugs to report, case closed.

I'll leave some notes for others who may stumble upon this issue in the future. My project's goal is to move deps from java8/boot 2.0.2 to java 17/boot 3.2.1. This is important because it seems that spring.sleuth.propagation.type: w3c was simply not respected by my app and JMS properties still were sent in the B3 format. boot 2.0.2 didn't have this spring property implemented yet.

So for anyone reading this - if the goal is to upgrade whatever 2.x version of spring to 3.x - first upgrade to 2.7.18 and stop wasting the time of the maintainers XD

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: messaging Issues in messaging modules (jms, messaging) theme: observability An issue related to observability and tracing type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests