opentracing-contrib / java-spring-messaging

OpenTracing Spring Messaging instrumentation
Apache License 2.0
3 stars 11 forks source link

Convert header to string if it is byte array #25

Closed pavolloffay closed 5 years ago

pavolloffay commented 5 years ago

Resolves #24

As the header type is object we don't know which codec should be used - text_map, binary. If the value is always byte array we could use Format.Builtin.BINARY when extracting/injecting the context.

Signed-off-by: Pavol Loffay ploffay@redhat.com

pavolloffay commented 5 years ago

@dreaderxxx could you please test this PR in your environment?

cc @gytis you could review :)

gytis commented 5 years ago

@pavolloffay is it possible for the byte[] value be anything other than string in this situation? E.g. serialized object. If not then this looks good to me.

pavolloffay commented 5 years ago

Honestly I don't know. I would say it can be anything. Would it fail on exception in this case?

gytis commented 5 years ago

Javadoc of this constructor says this: The behavior of this constructor when the given bytes are not valid in the default charset is unspecified.

gytis commented 5 years ago

So I believe that exception is a possibility

pavolloffay commented 5 years ago

My last try would be to wrap it into try-catch and fallback to String.valueOf. What do you think?

gytis commented 5 years ago

I think that would be better. Assuming it fixes issues with Kafka that @dreaderxxx was refering.

pavolloffay commented 5 years ago

@gytis I have updated the PR, could you please re-review?

@dreaderxxx would you like to test this PR in your environment?

dreaderxxx commented 5 years ago

I did some testing, initial exception is fixed. But I faced with another one

It is hard to explain in few words, it looks like if we inject spanContext as String into header(as it done now), it will cause another exception on kafka send - DefaultKafkaHeaderMapper:177(spring-kafka) converts String header value to byte[] back but with extra quotes added because of ObjectMapper and finally when tracer tries to extract spancontext next time it fails because of this extra quotes with exception

java.lang.NumberFormatException: For input string: ""5c"
    at java.lang.NumberFormatException.forInputString(NumberFormatException.java:65)
    at java.lang.Integer.parseInt(Integer.java:569)
    at java.math.BigInteger.<init>(BigInteger.java:470)
    at io.jaegertracing.SpanContext.contextFromString(SpanContext.java:124)
    at io.jaegertracing.propagation.TextMapCodec.extract(TextMapCodec.java:73)
    at io.jaegertracing.propagation.TextMapCodec.extract(TextMapCodec.java:27)
    at io.jaegertracing.PropagationRegistry$ExceptionCatchingExtractorDecorator.extract(PropagationRegistry.java:57)
    at io.jaegertracing.Tracer.extract(Tracer.java:193)
    at io.opentracing.util.GlobalTracer.extract(GlobalTracer.java:143)
    at io.opentracing.contrib.kafka.TracingKafkaUtils.extract(TracingKafkaUtils.java:41)
    at io.opentracing.contrib.kafka.TracingKafkaUtils.buildAndInjectSpan(TracingKafkaUtils.java:83)
    at io.opentracing.contrib.kafka.TracingProducerInterceptor.onSend(TracingProducerInterceptor.java:27)
    at org.apache.kafka.clients.producer.internals.ProducerInterceptors.onSend(ProducerInterceptors.java:61)
    at org.apache.kafka.clients.producer.KafkaProducer.send(KafkaProducer.java:767)
    at org.springframework.kafka.core.DefaultKafkaProducerFactory$CloseSafeProducer.send(DefaultKafkaProducerFactory.java:285)
    at org.springframework.kafka.core.KafkaTemplate.doSend(KafkaTemplate.java:349)
    at org.springframework.kafka.core.KafkaTemplate.send(KafkaTemplate.java:206)
    at org.springframework.integration.kafka.outbound.KafkaProducerMessageHandler.handleRequestMessage(KafkaProducerMessageHandler.java:384)
    at org.springframework.integration.handler.AbstractReplyProducingMessageHandler.handleMessageInternal(AbstractReplyProducingMessageHandler.java:109)
    at org.springframework.integration.handler.AbstractMessageHandler.handleMessage(AbstractMessageHandler.java:157)
    at org.springframework.integration.dispatcher.AbstractDispatcher.tryOptimizedDispatch(AbstractDispatcher.java:116)
    at org.springframework.integration.dispatcher.UnicastingDispatcher.doDispatch(UnicastingDispatcher.java:132)
    at org.springframework.integration.dispatcher.UnicastingDispatcher.dispatch(UnicastingDispatcher.java:105)
    at org.springframework.integration.channel.AbstractSubscribableChannel.doSend(AbstractSubscribableChannel.java:73)
    at org.springframework.integration.channel.AbstractMessageChannel.send(AbstractMessageChannel.java:463)
    at org.springframework.integration.channel.AbstractMessageChannel.send(AbstractMessageChannel.java:407)
    at org.springframework.messaging.core.GenericMessagingTemplate.doSend(GenericMessagingTemplate.java:181)
    at org.springframework.messaging.core.GenericMessagingTemplate.doSend(GenericMessagingTemplate.java:160)
    at org.springframework.messaging.core.GenericMessagingTemplate.doSend(GenericMessagingTemplate.java:47)
    at org.springframework.messaging.core.AbstractMessageSendingTemplate.send(AbstractMessageSendingTemplate.java:108)
    at org.springframework.integration.handler.AbstractMessageProducingHandler.sendOutput(AbstractMessageProducingHandler.java:426)
    at org.springframework.integration.handler.AbstractMessageProducingHandler.produceOutput(AbstractMessageProducingHandler.java:336)
    at org.springframework.integration.handler.AbstractMessageProducingHandler.sendOutputs(AbstractMessageProducingHandler.java:227)
    at org.springframework.integration.handler.AbstractReplyProducingMessageHandler.handleMessageInternal(AbstractReplyProducingMessageHandler.java:115)
    at org.springframework.integration.handler.AbstractMessageHandler.handleMessage(AbstractMessageHandler.java:157)
    at org.springframework.integration.dispatcher.AbstractDispatcher.tryOptimizedDispatch(AbstractDispatcher.java:116)
    at org.springframework.integration.dispatcher.UnicastingDispatcher.doDispatch(UnicastingDispatcher.java:132)
    at org.springframework.integration.dispatcher.UnicastingDispatcher.dispatch(UnicastingDispatcher.java:105)
    at org.springframework.integration.channel.AbstractSubscribableChannel.doSend(AbstractSubscribableChannel.java:73)
    at org.springframework.integration.channel.AbstractMessageChannel.send(AbstractMessageChannel.java:463)
    at org.springframework.integration.channel.AbstractMessageChannel.send(AbstractMessageChannel.java:407)
    at org.springframework.messaging.core.GenericMessagingTemplate.doSend(GenericMessagingTemplate.java:181)
    at org.springframework.messaging.core.GenericMessagingTemplate.doSend(GenericMessagingTemplate.java:160)
    at org.springframework.messaging.core.GenericMessagingTemplate.doSend(GenericMessagingTemplate.java:47)
    at org.springframework.messaging.core.AbstractMessageSendingTemplate.send(AbstractMessageSendingTemplate.java:108)
    at org.springframework.integration.endpoint.MessageProducerSupport.sendMessage(MessageProducerSupport.java:203)
    at org.springframework.integration.kafka.inbound.KafkaMessageDrivenChannelAdapter.access$300(KafkaMessageDrivenChannelAdapter.java:70)
    at org.springframework.integration.kafka.inbound.KafkaMessageDrivenChannelAdapter$IntegrationRecordMessageListener.onMessage(KafkaMessageDrivenChannelAdapter.java:387)
    at org.springframework.integration.kafka.inbound.KafkaMessageDrivenChannelAdapter$IntegrationRecordMessageListener.onMessage(KafkaMessageDrivenChannelAdapter.java:364)
    at org.springframework.kafka.listener.KafkaMessageListenerContainer$ListenerConsumer.doInvokeRecordListener(KafkaMessageListenerContainer.java:1071)
    at org.springframework.kafka.listener.KafkaMessageListenerContainer$ListenerConsumer.doInvokeWithRecords(KafkaMessageListenerContainer.java:1051)
    at org.springframework.kafka.listener.KafkaMessageListenerContainer$ListenerConsumer.invokeRecordListener(KafkaMessageListenerContainer.java:998)
    at org.springframework.kafka.listener.KafkaMessageListenerContainer$ListenerConsumer.invokeListener(KafkaMessageListenerContainer.java:866)
    at org.springframework.kafka.listener.KafkaMessageListenerContainer$ListenerConsumer.run(KafkaMessageListenerContainer.java:724)
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
    at java.util.concurrent.FutureTask.run$$$capture(FutureTask.java:266)
    at java.util.concurrent.FutureTask.run(FutureTask.java)
    at java.lang.Thread.run(Thread.java:748)
dreaderxxx commented 5 years ago

I made PR to fix this https://github.com/pavolloffay/java-spring-messaging/pull/1

pavolloffay commented 5 years ago

Thanks @dreaderxxx, does it work with your fix?

dreaderxxx commented 5 years ago

yes, @pavolloffay

pavolloffay commented 5 years ago

@geoand @gytis if this is fine with you could you please merge and cut a new release?

geoand commented 5 years ago

Looks good to me! Gytis, wdyt?

On Thu, Jan 10, 2019, 14:38 Pavol Loffay <notifications@github.com wrote:

@geoand https://github.com/geoand @gytis https://github.com/gytis if this is fine with you could you please merge and cut a new release?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/opentracing-contrib/java-spring-messaging/pull/25#issuecomment-453082082, or mute the thread https://github.com/notifications/unsubscribe-auth/AELBv2Wagh9Gx1SRPUOewvqHb-uTnv-Lks5vBzRVgaJpZM4Z13tC .

geoand commented 5 years ago

@gytis would you like to launch a new release?

gytis commented 5 years ago

Release is in progress

geoand commented 5 years ago

@gytis always one step ahead :)