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

Events should contain information about source channel #638

Closed
nicolaferraro opened this issue Nov 26, 2018 · 4 comments
Closed

Events should contain information about source channel #638

nicolaferraro opened this issue Nov 26, 2018 · 4 comments

Comments

@nicolaferraro
Copy link
Contributor

As discussed shortly in the Slack channel, in order to implement conditional logic at application level, events forwarded by a channel should contain information about the channel itself, so a application framework can dispatch them to the right handler.

This is useful in services that need to subscribe to multiple channels and treat events differently based on their origin.

A possible way to implement this is to use Cloudevents extensions that have a http binding mapping them to CE-X-Yyy headers.

We can use CE-X-KnativeChannel to set the source channel. I don't think there's need to track the history here, but only last one.

I can work on this.

@evankanderson
Copy link
Member

Would adding a URL path or other distinguishing routing information work for this purpose?

I'm slightly concerned about this pattern as it seems to violate a few of our design principles:

enable late-binding event sources and event consumers

If the consumer needs to know about the channels connected to it, it seems like this pushes the binding early (into the design phase), and makes the overall design more brittle.

Services can be connected to create new applications without modifying producer or consumer

This is a restatement of the above -- in this case, it feels like the consumer would need to be modified even for the same application architecture if different names are assigned to the routing path.

I'm willing to believe that there's a good use case for this, but I'd like to better understand the use case so we can consider multiple solutions to the problem -- this has worked well in other API design areas where the best solution ends up being inspired by someone else's thought.

@evankanderson
Copy link
Member

To copy some options from slack:

Enterprise Integration Patterns suggests a Message History header which would continue to grow over time. You could express this as a JSON or semicolon-separated history list which would be fairly compatible with HTTP and Cloud Events.

@nicolaferraro
Copy link
Contributor Author

nicolaferraro commented Nov 27, 2018

Yeah, I should have mentioned what kind of "application" I'm talking about.
I agree that services, especially functions, should be as much as possible agnostic on how they are invoked, in order to improve composability.

I'm currently experimenting with the "glue" code that brings together services and compose workflows. In particular I'm trying to bind EIP developed with Apache Camel to Knative. That kind of glue code cannot be agnostic about sources, channels and consumers it needs to connect.

The problem I'm presenting here arises when you need e.g.

  • to pair (zip) two events coming from two different channels into a single event
  • to merge two streams of events into a single one, applying distinct transformations on each side
  • ...

Distinguishing them by path is a option, but it does not seem possible with current API. Is it?

Btw, the channel header, also with message history, seems a much cleaner solution to me.

@evankanderson
Copy link
Member

evankanderson commented Nov 27, 2018 via email

nicolaferraro added a commit to nicolaferraro/eventing that referenced this issue Dec 11, 2018
nicolaferraro added a commit to nicolaferraro/eventing that referenced this issue Jan 7, 2019
nicolaferraro added a commit to nicolaferraro/eventing that referenced this issue Jan 7, 2019
nicolaferraro added a commit to nicolaferraro/eventing that referenced this issue Jan 7, 2019
nicolaferraro added a commit to nicolaferraro/eventing that referenced this issue Jan 7, 2019
nicolaferraro added a commit to nicolaferraro/eventing that referenced this issue Jan 10, 2019
knative-prow-robot pushed a commit that referenced this issue Jan 10, 2019
* Fix #638: add message history into header

* Fix #638: use simple hostname list and cloudevents format

* Fix #638: add new header to unit tests

* Fix #638: add space bewteen values and refactor tests

* Fix #638: fix codestyle
mgencur pushed a commit to mgencur/eventing that referenced this issue Jun 17, 2020
* fixing some script

Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>

* fix release script

Co-authored-by: Matthias Wessendorf <mwessend@redhat.com>
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

No branches or pull requests

2 participants