Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added capability to return generated eventid if an event has been dispatched after applying filters in the dispatch method #2867

Closed
wants to merge 7 commits into from

Conversation

vinsri-dev
Copy link

Checklist:

vinsri-dev and others added 7 commits September 30, 2023 13:56
… by the dispatch method in the central evening logic. This helps to extend the concrete implementations to log details like sqs message attributes only when dispatched for correlation.

Signed-off-by: Surya V <[email protected]>
Signed-off-by: Surya V <[email protected]>
…tion of event sources to log it for correlation

Signed-off-by: Surya V <[email protected]>
Signed-off-by: Surya V <[email protected]>
@vinsri-dev vinsri-dev changed the title Vinsri dev/master Added capability to return generated eventid if an event has been dispatched after applying filters in the dispatch method Oct 16, 2023
@vinsri-dev
Copy link
Author

vinsri-dev commented Oct 16, 2023

@whynowy

This a follow up to the PR : #2831
Closed the 2831 PR by an accident.

Based on your comments in 2831 PR
"
@whynowy :
**
Do we really need this? There's an option in the dispatch() function, to WithID(), which is used for tracking. See the Kafka example.

https://github.com/argoproj/argo-events/blob/master/eventsources/sources/kafka/start.go#L247
**

"

Still i think this code change might make sense for below reasons.

The idea is not only to get the event id, but also to be considered only if dispatched.

If the filter condition is not met (https://github.com/argoproj/argo-events/blob/master/eventsources/eventing.go#L533), we dont want to consider the event as dispatched since it is not dispatched to eventbus.

Please let me know your thoughts

Copy link
Contributor

This Pull Request is stale because it has been open for 60 days with
no activity. It will be closed in 7 days if no further activity.

@github-actions github-actions bot added the stale label Dec 16, 2023
@github-actions github-actions bot closed this Dec 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant