Delivery failure #1163

thomasjohansen · 2023-07-10T13:44:03Z

Changes

🎁 Add delivery failure (like for channels: https://knative.dev/docs/eventing/event-delivery/#configuring-channel-event-delivery)

/kind enhancement

Release Note

Added error information to failed messages

…append

linux-foundation-easycla · 2023-07-10T13:44:08Z

The committers listed above are authorized under a signed CLA.

✅ login: thomasjohansen / name: Thomas Johansen (52e3b07, 24c39af, 4250b09, 0371885, 51afdb7, 69ef0b3, 30fdc1b, 0c426b7, 095834a, faea709, d38fc3d, 6be7192, 5fb51ca, cf22797, 8db07c6, 4b1535f, 031cacc, 0c14e46, 4a69726, 29a2719, 7d49b80, 3589e6f, 114be75, 8e2b28a)
✅ login: gabo1208 / name: Gabriel Freites (e5e6abe, fcfc5ad, 4b38a8f, 2c761bb, b4852da, 107f3c3)

knative-prow · 2023-07-10T13:44:12Z

Welcome @thomasjohansen! It looks like this is your first PR to knative-sandbox/eventing-rabbitmq 🎉

pkg/dispatcher/dispatcher.go

codecov · 2023-07-15T14:07:10Z

Codecov Report

Merging #1163 (44c9f9f) into main (2e216ec) will increase coverage by 0.37%.
Report is 19 commits behind head on main.
The diff coverage is 47.65%.

❗ Current head 44c9f9f differs from pull request most recent head 107f3c3. Consider uploading reports for the commit 107f3c3 to get more accurate results

@@            Coverage Diff             @@
##             main    #1163      +/-   ##
==========================================
+ Coverage   74.22%   74.59%   +0.37%     
==========================================
  Files          44       44              
  Lines        3309     3417     +108     
==========================================
+ Hits         2456     2549      +93     
- Misses        764      767       +3     
- Partials       89      101      +12

Files Changed	Coverage Δ
pkg/rabbit/rabbit_mocks.go	`78.37% <ø> (+11.71%)`	⬆️
pkg/dispatcher/dispatcher.go	`50.82% <36.88%> (+8.75%)`	⬆️
pkg/reconciler/trigger/trigger.go	`63.19% <80.00%> (+0.18%)`	⬆️
pkg/reconciler/broker/broker.go	`86.12% <100.00%> (+0.04%)`	⬆️
pkg/reconciler/broker/resources/dispatcher.go	`94.15% <100.00%> (+0.24%)`	⬆️
pkg/reconciler/trigger/resources/dispatcher.go	`95.02% <100.00%> (+0.37%)`	⬆️

... and 3 files with indirect coverage changes

pkg/dispatcher/dispatcher.go

pkg/reconciler/trigger/resources/dispatcher.go

pkg/dispatcher/dispatcher.go

…ed in the trigger, if its not defined in the trigger then in the broker, and if neither is defined then they are not rerouted

gabo1208 · 2023-07-24T17:53:28Z

pkg/dispatcher/dispatcher.go

-		err = msg.Nack(false, false)
-		if err != nil {
-			logging.FromContext(ctx).Warn("failed to NACK event: ", err)
+		if event == nil {


I think we don't need this if, the event probably is going to be nill anyways, so just set the headers and to the else part

…se its always going to be nil

… stops when a context cancel gets there

knative-prow · 2023-07-31T17:49:00Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gabo1208, thomasjohansen

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [gabo1208]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

gabo1208 · 2023-07-31T18:30:35Z

/cherry-pick release-1.11

knative-prow-robot · 2023-07-31T18:31:21Z

@gabo1208: new pull request could not be created: failed to create pull request against knative-sandbox/eventing-rabbitmq#release-1.11 from head knative-prow-robot:cherry-pick-1163-to-release-1.11: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","field":"head","code":"invalid"}],"documentation_url":"https://docs.github.com/rest/pulls/pulls#create-a-pull-request"}

In response to this:

/cherry-pick release-1.11

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

gabo1208 · 2023-07-31T18:37:07Z

/cherry-pick release-1.11

knative-prow-robot · 2023-07-31T18:37:46Z

@gabo1208: new pull request created: #1187

In response to this:

/cherry-pick release-1.11

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

gabo1208 · 2023-07-31T18:55:40Z

/cherry-pick release-1.11

knative-prow-robot · 2023-07-31T18:56:19Z

@gabo1208: new pull request created: #1188

In response to this:

/cherry-pick release-1.11

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

thomasjohansen and others added 5 commits July 7, 2023 12:46

Add knativeerrordest to the event extension when delivery failure

52e3b07

Check if the extension is set, else set it. If not act like nothing h…

24c39af

…append

Cleanup and requeue variable

4250b09

Merge branch 'knative-sandbox:main' into delivery-failure

0371885

Set hedders in msg and requeue

51afdb7

knative-prow bot added the kind/enhancement label Jul 10, 2023

knative-prow bot requested review from odacremolbap and zroubalik July 10, 2023 13:44

knative-prow bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 10, 2023

Lint

69ef0b3

gabo1208 reviewed Jul 10, 2023

View reviewed changes

pkg/dispatcher/dispatcher.go Outdated Show resolved Hide resolved

gabo1208 reviewed Jul 10, 2023

View reviewed changes

pkg/dispatcher/dispatcher.go Outdated Show resolved Hide resolved

thomasjohansen added 3 commits July 10, 2023 22:46

Wrong place to set headers

30fdc1b

Forgot the status code

0c426b7

Nack if there's a header

095834a

Send event directly to RabbitMQ

faea709

gabo1208 reviewed Jul 16, 2023

View reviewed changes

pkg/dispatcher/dispatcher.go Outdated Show resolved Hide resolved

gabo1208 reviewed Jul 16, 2023

View reviewed changes

pkg/dispatcher/dispatcher.go Outdated Show resolved Hide resolved

thomasjohansen added 2 commits July 16, 2023 20:56

Handle error and send an event for it

d38fc3d

Add functions to handle DLQ in the new way

6be7192

knative-prow bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 17, 2023

gabo1208 suggested changes Jul 17, 2023

View reviewed changes

pkg/reconciler/trigger/resources/dispatcher.go Show resolved Hide resolved

pkg/dispatcher/dispatcher.go Outdated Show resolved Hide resolved

knative-prow bot assigned gabo1208 Jul 17, 2023

thomasjohansen added 3 commits July 17, 2023 14:03

Moved things around

5fb51ca

Added missing arg

cf22797

We should always Ack in dispatch to DLQ

8db07c6

thomasjohansen and others added 2 commits July 24, 2023 19:47

Quick push

7d49b80

now the failed messages are directly routed to the DLQ exchange defin…

e5e6abe

…ed in the trigger, if its not defined in the trigger then in the broker, and if neither is defined then they are not rerouted

gabo1208 reviewed Jul 24, 2023

View reviewed changes

gabo1208 mentioned this pull request Jul 18, 2023

Add metadata to Dead lettered Messages #833

Closed

2 tasks

Update test. Still failing

3589e6f

gabo1208 force-pushed the delivery-failure branch from ac02b62 to 50752ba Compare July 27, 2023 15:13

gabo1208 and others added 4 commits July 27, 2023 17:16

fixed tests and removed event nil case when binding.ToEvent fails cau…

fcfc5ad

…se its always going to be nil

Added two simple test, to test the dlq function

114be75

fixed unit tests

4b38a8f

this is just an smoke test for consumefromqueue function to see if it…

2c761bb

… stops when a context cancel gets there

gabo1208 force-pushed the delivery-failure branch from 50752ba to 2c761bb Compare July 27, 2023 15:20

gabo1208 and others added 2 commits July 27, 2023 22:11

added defered confirmation to connection mock

b4852da

Extended tests

8e2b28a

knative-prow bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 31, 2023

improving coverage

107f3c3

gabo1208 force-pushed the delivery-failure branch from 44c9f9f to 107f3c3 Compare July 31, 2023 17:36

gabo1208 approved these changes Jul 31, 2023

View reviewed changes

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jul 31, 2023

knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 31, 2023

knative-prow bot merged commit 06ad50d into knative-extensions:main Jul 31, 2023
19 checks passed

knative-prow-robot mentioned this pull request Jul 31, 2023

[release-1.11] Delivery failure #1187

Closed

knative-prow-robot mentioned this pull request Jul 31, 2023

[release-1.11] Delivery failure #1188

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delivery failure #1163

Delivery failure #1163

thomasjohansen commented Jul 10, 2023 •

edited

Loading

linux-foundation-easycla bot commented Jul 10, 2023 •

edited

Loading

knative-prow bot commented Jul 10, 2023

codecov bot commented Jul 15, 2023 •

edited

Loading

gabo1208 Jul 24, 2023

knative-prow bot commented Jul 31, 2023

gabo1208 commented Jul 31, 2023

knative-prow-robot commented Jul 31, 2023

gabo1208 commented Jul 31, 2023

knative-prow-robot commented Jul 31, 2023

gabo1208 commented Jul 31, 2023

knative-prow-robot commented Jul 31, 2023

Delivery failure #1163

Delivery failure #1163

Conversation

thomasjohansen commented Jul 10, 2023 • edited Loading

Changes

linux-foundation-easycla bot commented Jul 10, 2023 • edited Loading

knative-prow bot commented Jul 10, 2023

codecov bot commented Jul 15, 2023 • edited Loading

Codecov Report

gabo1208 Jul 24, 2023

Choose a reason for hiding this comment

knative-prow bot commented Jul 31, 2023

gabo1208 commented Jul 31, 2023

knative-prow-robot commented Jul 31, 2023

gabo1208 commented Jul 31, 2023

knative-prow-robot commented Jul 31, 2023

gabo1208 commented Jul 31, 2023

knative-prow-robot commented Jul 31, 2023

thomasjohansen commented Jul 10, 2023 •

edited

Loading

linux-foundation-easycla bot commented Jul 10, 2023 •

edited

Loading

codecov bot commented Jul 15, 2023 •

edited

Loading