Flat headers #2613
Flat headers #2613
Conversation
ramonsmits
force-pushed
the
flat-headers
branch
from
175dbb1
to
1ec130a
Compare
…for publicly visible type or member 'TransportOperationExt'
|
|
||
| if (flatHeaders) | ||
| { | ||
| foreach (var i in transportOperation.Message.Headers) |
There was a problem hiding this comment.
@ramonsmits you mention the limit of 10. What would happen if the collection here is bigger than 10?
There was a problem hiding this comment.
@SzymonPobiega Likely we'll get a HTTP 4xx error but not sure
There was a problem hiding this comment.
OK. I know we discussed this and I didn't raise a flag but I thought about it more and maybe a better option (more in-line with the other transports we touched) would be to have an API that allows the user to register a callback here that is in form of:
Func<Dictionary<string,string>, List<MessageAttributeValue>
The default value of that callback would do what we do (put all the headers in a single attribute) but users would be able to override it. This way we require the users to think about the mapping and explicitly take control over it.
There was a problem hiding this comment.
I agree with @SzymonPobiega here. When the native integration feature was improved to support what it supports today, we spent a lot of time thinking about a possible "promote headers" feature that would have allowed users to promote specific headers selectively to be message attributes. Considering that the transport has no control over the number of headers, this could blow up unexpectedly upon a Core change that adds new headers that suddenly overflow the amount of allowed message attributes.
I'd even argue that there should be no default behavior, and the user must always consciously select which headers should be promoted to avoid any surprise with subsequent releases.
There was a problem hiding this comment.
@SzymonPobiega yes, discussed such a similar callback. Problem with SQS is that as mentioned on Slack that there are multiple native messages types due to the usage of SNS and SQS and batched/non-batched. A callback for headers which is shared between all is a good alternative but... there are 2 options:
- When registered, only use that and replace are header serializer
- When registered, invoke after our header serializer
Benefit of (1) is that it skips our header serialization which could be undone by changes made in the callback. In our use-case we could remove that header
There was a problem hiding this comment.
When the native integration feature was improved to support what it supports today, we spent a lot of time thinking about a possible "promote headers" feature that would have allowed users to promote specific headers selectively to be message attributes. ....... I'd even argue that there should be no default behavior, and the user must always consciously select which headers should be promoted to avoid any surprise with subsequent releases.
This is similar to MT which uses envelopes. The transport can decide which header is puts in the header part of the serialized envelope of the payload and which headers to put in the native headers. In that regard MT is more flexible as it would allow to register a custom serializer with specific behavior.
its a cross transport concern and we already have issues in NServiceBus core repo on envelope support
…ey to MessageDispatcher to be similar in design as other transports.
It would be helpful to provide more context about the why of:
|
@mauroservienti I've updated it to: "However, for native integration purposes is can be useful to set the headers so that the message format conforms to the receiver contract." better? |
NServiceBus serialized all headers into a single native header to workaround the 10 header limit. However, for native integration purposes is can be useful to set the headers so that the message format conforms to the receiver contract.