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 #2831

Closed
wants to merge 7 commits into from

Conversation

vinsri-dev
Copy link

@vinsri-dev vinsri-dev commented Sep 29, 2023

Added logic in the dispatch process to return the generated eventid by the eventing process, whether if the event has been dispatched or not post applying filters. If the event is not dispatched post applying filters, the event id is empty.

This flag helps to handle certain aspects like to log if the event has been dispatched.

Added log function in the sqs to capture the message attributes only if the event is dispatched. This logging helps to correlate the eventid generated in eventbus with the upstream message attributes for end to end correlation and tracing
Checklist:

@vinsri-dev vinsri-dev changed the title Added capability to return true/false if an event has been dispatched after applying filters in the dispatch method Added capability to return generated eventid if an event has been dispatched after applying filters in the dispatch method Sep 30, 2023
vinsri-dev and others added 6 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
Copy link
Author

@VaibhavPage , @whynowy : Could you please review and let me know your thoughts.

Thanks

@vinsri-dev
Copy link
Author

@VaibhavPage @whynowy
Can you please help me comment if i missed anything for this PR. Could you please help progress.

@whynowy
Copy link
Member

whynowy commented Oct 14, 2023

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

@vinsri-dev vinsri-dev closed this by deleting the head repository Oct 16, 2023
@vinsri-dev
Copy link
Author

@whynowy : Sorry by an accident i deleted my forked repository.

Anyways, i can resume it.
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants