Enrich failed events with custom attributes provided via HTTP Response Headers

[danielezonca]

Problem
Currently deadLetterSink strategy enriches error message with a set of predefined attributes (knativeerrordest, knativeerrorcode and knativeerrordata).
It is not possible to provide additional custom attributes to add to the error and the only attribute where there is a bit of control is knativeerrordata but it contains the body of the response so it is a single value.
The proposal is to have a mechanism (opt-in?) to ask Knative Broker to take HTTP Response Headers and add them as CloudEvents attributes to the deadLetterSink event.
A proposal for the opt-in scenario could be to have an allowlist annotation defined at Trigger (or Broker?) level with the list of supported custom extensions and * to forward all of them

# enable two custom extensions
x-kafka.eventing.knative.dev/deadLetterSink.custom.extensions: ['mycustomattribute', 'anothercustomattribute']
# enable all extensions
x-kafka.eventing.knative.dev/deadLetterSink.custom.extensions: ['*']

Persona:
I think this feature is useful both for Event consumer (developer) and System Operator personas (essentially all personas involved with error handling)

Exit Criteria
It should be possible for a subscriber sink to enrich error HTTP Response with custom HTTP headers that are included in the deadLetterSink message

Time Estimate (optional):
This is probably a quite simple implementation in Knative Kafka Broker (it changes the same classes of this other PR #2374) so it could be probably also a good-first-issue (1-2 days). It might be useful to bring a similar requirement to Knative Eventing to generalise the solution but it can takes more time.

Additional context (optional)
We have an error catalog (that is not based on HTTP error codes) that has error id, error reason and maybe also nested errors and we would like to avoid to "encode" them as raw string in the HTTP Response body.

Example

Event from Broker to Subscriber

HTTP Headers

specversion: 1.0
type: com.github.pull_request.opened
source: https://github.com/cloudevents/spec/pull
subject: 123
id: A234-1234-1234
time: 2018-04-05T17:31:00Z
comexampleextension1: value

Payload

My payload

Subscriber Response

HTTP Response

status: 500
mycustomerrordefinition: maintenance window

HTTP Response Body

Internal server error

DeadLetterSink message

Attributes

specversion: 1.0
type: com.github.pull_request.opened
source: https://github.com/cloudevents/spec/pull
subject: 123
id: A234-1234-1234
time: 2018-04-05T17:31:00Z
comexampleextension1: value
mycustomerrordefinition: maintenance window
knativeerrordest: /destination
knativeerrorcode: 500
knativeerrordata: Internal server error

Body

My payload

[matzew]

@danielezonca Thanks for the feedback!

I have a question:
Perhaps we could go with some well defined prefix? If present in the headers, those would be passed downstream as CE extension?

IMO this lowers the burden of extra configuration?

[danielezonca]

@danielezonca Thanks for the feedback!

I have a question: Perhaps we could go with some well defined prefix? If present in the headers, those would be passed downstream as CE extension?

IMO this lowers the burden of extra configuration?

Do you mean something like a fixed prefix ce-ext-*?
Yes this is a possible option, I was thinking about keeping this more flexible so that it makes less assumption on subscriber side and it can be easily configured but if the prefix is configurable at subscriber level it is probably enough 🤔

[egorlepa]

I believe this should be brought to general Knative Eventing spec first. So that interface(spec) - implementation relationship is preserved. Changes should be pushed downstream, not the other way around. If it makes sense :)

[danielezonca]

Hi @egorlepa I agree that this is a good candidate for a knative eventing improvement.
We faced this need using the Knative Kafka Broker but it is definitely not specific for Kafka.
I started this discussion with @matzew and he suggested to bring this upstream but I'm not familiar with Knative Eventing spec process so I dumped the idea here.

Fine for me to target spec first :)

[matzew]

Let's have it on the Eventing WG call, today (Wednesday)

[matzew]

We are using the kne- as a prefix.

If the response contains headers that start with the prefix, such as kne-mycustomerrordefinition: maintenance window, they end up as CloudEvent extension headers, like:

☁️  cloudevents.Event
Validation: valid
Context Attributes,
  specversion: 1.0
  type: demo
  source: /my/curl/command
  id: 4711
  datacontenttype: application/json
Extensions,
  mycustomerrordefinition: maintenance window
  knativeerrorcode: 404
  ...

Here is a e2e test that shows the pattern: https://github.com/knative-sandbox/eventing-kafka-broker/pull/2657/files#diff-0de2d5afb422b7e648e6a13010e4331ecad2c15de240dcea0e8177e263e73e10R391-R422