opentracing-contrib / java-spring-messaging

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

Fix timestamp removal #17

Closed svenwltr closed 6 years ago

svenwltr commented 6 years ago

The current approach removes the timestamp and id header, because the MessageHeaderAccessor.copyHeaders will not copy readOnly headers. Using the MessageBuilder will ensure that all headers from the original message, including id/timestamp will also be in the result message.

svenwltr commented 6 years ago

I do not know how to fix the licence header error. We did not touch the headers and they are actually there. We had to change the year.

bjoernhaeuser commented 6 years ago

Hello everyone,

we wanted to test the opentracing for spring messaging. Though the current implementation is removing the timestamp of incoming messages. The timestamp is required for grouping of messages.

We want to get the discussion about this started. The implementation feels a bit sketchy and especially the shouldKeepHeadersMutable test shows that the header should be mutable.

In our opinion we should try to make the headers immutable and copy the original id + timestamp.

What do you think?

bjoernhaeuser commented 6 years ago

@gytis do you mind taking a look here? :)

gytis commented 6 years ago

Looks good. Thanks for the contribution and sorry for a delay...