JMS Context properties 'X-B3-TraceId' is not a valid java identifier. #4202

tw-atroehrsm · 2023-10-10T16:35:55Z

Describe the bug

Exception when publising a JMS message. Not all characters are allowed according to JMS naming standard, i.e hyphen.

	
jakarta.jms.JMSRuntimeException: AMQ139012: The property name 'X-B3-TraceId' is not a valid java identifier.

	at org.apache.activemq.artemis.jms.client.ActiveMQMessage.checkProperty(ActiveMQMessage.java:911)
	at org.apache.activemq.artemis.jms.client.ActiveMQMessage.setStringProperty(ActiveMQMessage.java:669)
	at io.micrometer.core.instrument.binder.jms.JmsPublishObservationContext.lambda$new$0(JmsPublishObservationContext.java:40)
	at io.micrometer.tracing.handler.PropagatingSenderTracingObservationHandler.lambda$onStart$0(PropagatingSenderTracingObservationHandler.java:60)
	at brave.propagation.B3Propagation$Format$1.inject(B3Propagation.java:46)

Environment

Micrometer version [e.g. 1.7.1] 1.12.0-M3
Micrometer registry [e.g. prometheus] prometheus
OS: [e.g. macOS] macOS
Java version: [e.g. output of java -version]

openjdk 17.0.6 2023-01-17
OpenJDK Runtime Environment Temurin-17.0.6+10 (build 17.0.6+10)
OpenJDK 64-Bit Server VM Temurin-17.0.6+10 (build 17.0.6+10, mixed mode, sharing)

To Reproduce
How to reproduce the bug:

Use Spring Boot 3.2.0-M3 with artemis-starter and tracing enabled. Publish a message.

Expected behavior
Escape / Unescape Property Names to comply with JMS Java Identifier Rules
https://camel.apache.org/components/4.0.x/jms-component.html#_message_header_mapping

Additional context

The text was updated successfully, but these errors were encountered:

tw-atroehrsm · 2023-10-10T16:38:53Z

micrometer/micrometer-jakarta9/src/main/java/io/micrometer/jakarta9/instrument/jms/JmsProcessObservationContext.java

Line 39 in 1f2b1de

return message.getStringProperty(key);

micrometer/micrometer-jakarta9/src/main/java/io/micrometer/jakarta9/instrument/jms/JmsPublishObservationContext.java

Line 40 in 1f2b1de

message.setStringProperty(key, value);

would it be enough to apply escaping here? if yes I could prepare a PR

tw-atroehrsm · 2023-10-10T16:40:51Z

https://issues.apache.org/jira/browse/ARTEMIS-2700 some issue from 2020 where someone stumbled upon this.

Prior to this commit, both `JmsPublishObservationContext` and `JmsProcessObservationContext` would set propagation headers as String properties on the JMS `Message`. This would cause issues with JMS implementations, as the JMS specification only allows valid Java identifiers as keys. Typically, a `X-B3-TraceId` header fails this requirement as it contains "-" characters. This commit ensures that the following characters are escaped when publishing messages, and unescaped when receiving messages: * "-" becomes "_HYPHEN_" * "." becomes "_DOT_" Closes micrometer-metricsgh-4202

shakuzen · 2023-10-11T08:56:50Z

would it be enough to apply escaping here?

It would if it were standardized what escaping/unescaping should be used. Since it is not, whatever escaping we apply would assume the other side of the messaging is instrumented with something that applies the same escaping. I don't think that's a reasonable assumption unless the entire JMS ecosystem has adopted what Camel does.

There is a long history of this issue, and I think the only robust way around it is to avoid it entirely by using a propagation format that doesn't have hyphens in the header name. b3 single header format should work, as well as the W3C format. Is there any issue for you to switch to one of those instead of the B3 multi header?

For some history, see openzipkin/brave#584, openzipkin/brave#1019.

tw-atroehrsm · 2023-10-23T11:15:09Z

Our current infrastructure used the old headers. Im not sure what happend if we switch from B3-* to b3 single header. Any experiences with migration?

tw-atroehrsm · 2023-10-23T11:15:51Z

Otherwise I would be happy if the escaping would just work within the SB3 world.. as thats the only place we use JMS

shakuzen · 2023-10-24T08:49:23Z

Im not sure what happend if we switch from B3-* to b3 single header. Any experiences with migration?

You should ensure that all the components in your system can read B3 multi and b3 single headers, regardless of which they are configured to write. That way tracing still works during the transition phase from multi to single. You mentioned Spring Boot 3. If all of your traced components are SB3 applications, you may be interested in the properties mentioned in spring-projects/spring-boot#35611, which should allow for relatively easy migration.

Otherwise I would be happy if the escaping would just work within the SB3 world.. as thats the only place we use JMS

I appreciate your concern is that the issue you're facing gets resolved for your use case. At the low level of libraries like Micrometer, we have to consider all supported use cases. Micrometer is used outside of Spring Boot. It would also be a valid use case for a Spring Boot app using Micrometer to produce a message on a JMS broker that then gets consumed by an application written in a different language. Tracing wouldn't work with the proposed changes unless that application applied the same logic, despite not using Micrometer.

Something we could look into is overriding the configured propagation format when writing to or reading from message brokers, knowing the limitations of headers on some of them. I believe Brave did and/or does this. We'd need to look into this more.

github-actions · 2023-12-20T01:42:27Z

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

github-actions · 2023-12-27T01:47:34Z

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open.

Saljack · 2024-01-02T14:35:58Z

This is a problem because without escaping we cannot use B3 anymore. Because there is also X-B3-Sampled and it is not also valid property name. The problem started with upgrading to Spring Boot 3.2.1 which enables JMS instrumentation on JMS Listeners. If you send a message without any tracing header, it tries to use W3C and then B3 and fails because - is an invalid character.

I don't know which project can fix it correctly because it can be fixed in Brave, Micrometer or Spring Boot. From my point of view some escaping (or catching the IllegalArgumentException in JmsProcessObservationContext) is better than failing to receive the message because of tracing.

So we have to use this configuration in Spring Boot:

management.tracing.propagation.consume=W3C

jonatan-ivanov · 2024-01-03T17:11:47Z

B3-single should also work, see this: #4202 (comment) and https://github.com/openzipkin/b3-propagation?tab=readme-ov-file#single-header

Saljack · 2024-01-03T18:31:02Z

It works if you send the b3 header but if there is not any tracing header then it reach this:
https://github.com/openzipkin/brave/blob/70600a5b035d5f83a184ba9db897d4656edd61fa/brave/src/main/java/brave/propagation/B3Propagation.java#L278
Where it tries to extract X-B3-Sampled and it fails because of -.

jonatan-ivanov · 2024-01-03T18:40:06Z

X-B3-Sampled is not B3-single, it is B3-multi, please check the second link in my previous comment.

Saljack · 2024-04-17T13:07:40Z

But the problem is there is no tracing header. So you send a message without any tracing and a processing service with observation tries to extract B3 headers and also tries to extract X-B3-Sampled. If there is any b3 header or any W3C header then it works.

tw-atroehrsm added the waiting-for-triage label Oct 10, 2023

tw-atroehrsm mentioned this issue Oct 10, 2023

Add observability support for JMS spring-projects/spring-framework#30335

Closed

bclozel mentioned this issue Oct 10, 2023

Escape/Unescape JMS String properties in contexts #4203

Closed

shakuzen removed the waiting-for-triage label Oct 11, 2023

shakuzen added the waiting for feedback We need additional information before we can continue label Oct 11, 2023

jonatan-ivanov linked a pull request Oct 13, 2023 that will close this issue

Escape/Unescape JMS String properties in contexts #4203

Closed

github-actions bot added the feedback-reminder label Dec 20, 2023

github-actions bot added the closed-as-inactive label Dec 27, 2023

github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 27, 2023

jonatan-ivanov removed waiting for feedback We need additional information before we can continue feedback-reminder labels Jan 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JMS Context properties 'X-B3-TraceId' is not a valid java identifier. #4202

JMS Context properties 'X-B3-TraceId' is not a valid java identifier. #4202

tw-atroehrsm commented Oct 10, 2023 •

edited

Loading

tw-atroehrsm commented Oct 10, 2023

tw-atroehrsm commented Oct 10, 2023

shakuzen commented Oct 11, 2023

tw-atroehrsm commented Oct 23, 2023

tw-atroehrsm commented Oct 23, 2023

shakuzen commented Oct 24, 2023

github-actions bot commented Dec 20, 2023

github-actions bot commented Dec 27, 2023

Saljack commented Jan 2, 2024

jonatan-ivanov commented Jan 3, 2024

Saljack commented Jan 3, 2024

jonatan-ivanov commented Jan 3, 2024

Saljack commented Apr 17, 2024

JMS Context properties 'X-B3-TraceId' is not a valid java identifier. #4202

JMS Context properties 'X-B3-TraceId' is not a valid java identifier. #4202

Comments

tw-atroehrsm commented Oct 10, 2023 • edited Loading

tw-atroehrsm commented Oct 10, 2023

tw-atroehrsm commented Oct 10, 2023

shakuzen commented Oct 11, 2023

tw-atroehrsm commented Oct 23, 2023

tw-atroehrsm commented Oct 23, 2023

shakuzen commented Oct 24, 2023

github-actions bot commented Dec 20, 2023

github-actions bot commented Dec 27, 2023

Saljack commented Jan 2, 2024

jonatan-ivanov commented Jan 3, 2024

Saljack commented Jan 3, 2024

jonatan-ivanov commented Jan 3, 2024

Saljack commented Apr 17, 2024

tw-atroehrsm commented Oct 10, 2023 •

edited

Loading