ISSUE: TM Function does not handle inner reference response

[nkreiger]

Per the triggermesh docs ->

"Functions may be used to implement custom event flow logic"

Expectation:

Using a function to do a custom/complex transformation before reaching an inner processing ksvc will perform the transformation and return the result of the ksvc.

Reality:

It does the custom transformation, but the response is nil.

https://github.com/triggermesh/aws-custom-runtime/blob/main/pkg/sender/sender.go#L56

Simple Example

# perform modification (only user code part) to attestation input
apiVersion: extensions.triggermesh.io/v1alpha1
kind: Function
metadata:
  name: custom-tsf
  namespace: default
spec:
  runtime: python
  public: true
  entrypoint: main
  sink:
    ref:
      name: example-inner
      namespace: default
      kind: Service
      apiVersion: serving.knative.dev/v1
  code: |
    def main(payload, context):
        payload['metdata'] = 'some change'

        return payload

hitting this function will always return nil, because it sends to an inner ksvc, I do not think you should have to create a sinkbinding or assume the broker for the inner ksvc each time to return the event...I think it should match the flow of a transformation which returns it to the same broker the transformation reads from (same with a filter).

It just seems like the function is unique in this regard, and it breaks expectations.

[antoineco]

@nkreiger I would suggest using the building blocks of Knative eventing here, and fronting the Kn Service with a Channel to make that flow truly asynchronous.

1. In your transformation, send to the Channel, not the Kn Service.
2. Create a Subscription that sends messages from the Channel to the Kn Service, and set the reply.ref attribute on that Subscription to wherever the response should be routed.

This is more portable: it applies to anything, custom and non-custom components, whereas the suggested change only applies to Function objects.

Our authoring tool https://github.com/triggermesh/til does that automatically for you, but if you manipulate YAML directly you need to interpolate those two objects yourself.

[nkreiger]

@nkreiger I would suggest using the building blocks of Knative eventing here, and fronting the Kn Service with a Channel to make that flow truly asynchronous.

1. In your transformation, send to the Channel, not the Kn Service.
2. Create a Subscription that sends messages from the Channel to the Kn Service, and set the replyTo attribute on that Subscription to wherever the response should be routed.

Our authoring tool https://github.com/triggermesh/til does that automatically for you, but if you manipulate YAML directly you need to interpolate those two objects yourself.

what about the use case where you don't explicitly know where you want the event to go (declaratively), so you want it just transformed, processed, and then returned to the source it originated from (ex. broker -> trigger -> function -> service -> broker) where the source would be the broker?

I closed my PR because it doesn't serve a cloud events API, so I can see how my PR does not solve that scenario.

Is there a use case for refactoring the aws-custom-runtime to serve a cloud events API instead of HTTP to allow for that chain of events, or am I missing the solution in your suggestion?

[antoineco]

Example:

# my-bridge.brg.hcl

router content_based "dispatch" {
  route {
    to = transformer.custom_tsf
  }
}

transformer function "custom_tsf" {
  runtime = "python"

  code = <<EOF
   def main(payload, context):
        payload['metdata'] = 'some change'

        return payload
  EOF

  ce_context {
    type = "my.transformation.v1.event"
  }

  public = true

  to = target.example_inner
}

target container "example_inner" {
  image = "registry.example.com/myapp:v1"

  reply_to = router.dispatch
}

Generated YAML. Notice the Channel and Subscription objects:

apiVersion: extensions.triggermesh.io/v1alpha1
kind: Function
metadata:
  labels:
    bridges.triggermesh.io/id: til_generated
  name: custom-tsf
spec:
  ceOverrides:
    extensions:
      type: my.transformation.v1.event
  code: |2
      def main(payload, context):
          payload['metdata'] = 'some change'
          return payload
  entrypoint: main
  public: true
  runtime: python
  sink:
    ref:
      apiVersion: messaging.knative.dev/v1
      kind: Channel
      name: example-inner
---
apiVersion: eventing.knative.dev/v1
kind: Broker
metadata:
  labels:
    bridges.triggermesh.io/id: til_generated
  name: dispatch
---
apiVersion: eventing.knative.dev/v1
kind: Trigger
metadata:
  labels:
    bridges.triggermesh.io/id: til_generated
  name: dispatch-r0
spec:
  broker: dispatch
  subscriber:
    ref:
      apiVersion: extensions.triggermesh.io/v1alpha1
      kind: Function
      name: custom-tsf
---
apiVersion: serving.knative.dev/v1
kind: Service
metadata:
  labels:
    bridges.triggermesh.io/id: til_generated
    networking.knative.dev/visibility: cluster-local
  name: example-inner
spec:
  template:
    spec:
      containers:
      - image: registry.example.com/myapp:v1
---
apiVersion: messaging.knative.dev/v1
kind: Channel
metadata:
  labels:
    bridges.triggermesh.io/id: til_generated
  name: example-inner
---
apiVersion: messaging.knative.dev/v1
kind: Subscription
metadata:
  labels:
    bridges.triggermesh.io/id: til_generated
  name: example-inner
spec:
  channel:
    apiVersion: messaging.knative.dev/v1
    kind: Channel
    name: example-inner
  reply:
    ref:
      apiVersion: eventing.knative.dev/v1
      kind: Broker
      name: dispatch
  subscriber:
    ref:
      apiVersion: serving.knative.dev/v1
      kind: Service
      name: example-inner

[nkreiger]

genius, thanks

[antoineco]

what about the use case where you don't explicitly know where you want the event to go

Fair point, if you don't know at design time where responses should be sent, my solution falls short.

But isn't that a bit dangerous? Nothing guarantees you that what's immediately before the transformation is a Broker, right? What if it's another custom service, or an event source. What does it do with the response? Most likely that response will be ignored.

To me, it sounds like the suggested approach works in one case, on one case only: when the sender is actually a Broker. Like I said, any other component would drop the response.

[antoineco]

PS: I didn't mean to discourage you! If the PR you sent solves your problem, we should work towards accepting it 👍

[nkreiger]

To me, it sounds like the suggested approach works in one case, on one case only: when the sender is actually a Broker. Like I said, any other component would drop the response.

That's why I touched on the possibility of making it a configuration in the spec so that you can turn it on for specific use cases...however, the solution you dropped I think solves this, because if you know the broker -> trigger -> function portion then you can assume the destination back declaratively, which is better as you point out above

...I think there may still be a corner case here or there where you would want the response directly from the function if you are integrating from non-serverless to serverless

ex. I have a microservice that hits a ksvc and I want to stand up a function to validate the input payload...so then it would go function -> ksvc, and the microservice expects a response...so maybe my PR solves that issue?

maybe ^ justifies the pr, thoughts?

[antoineco]

Potentially it does yes. In fact, we considered that exact use case already, and @tzununbekov implemented a component called Synchronizer, which is already part of TriggerMesh since v1.14.0 or so.

This component is placed in front of a Broker. It marks incoming events with a correlation ID, and keeps client connections open until the Broker receives an event with the original correlation ID.

With this component interpolated between the microservice and the broker, your microservice doesn't have to be aware of the asynchronous system located behind the broker, all it sees is a synchronous response originating from the Kn Service, but routed transparently through the Broker+Synchronizer.

Unfortunately we haven't documented that component thoroughly yet.

[nkreiger]

Potentially it does yes. In fact, we considered that exact use case already, and @tzununbekov implemented a component called Synchronizer, which is already part of TriggerMesh since v1.14.0 or so.

This component is placed in front of a Broker. It marks incoming events with a correlation ID, and keeps client connections open until the Broker receives an event with the original correlation ID.

With this component interpolated between the microservice and the broker, your microservice doesn't have to be aware of the asynchronous system located behind the broker, all it sees is a synchronous response originating from the Kn Service, but routed transparently through the Broker+Synchronizer.

Unfortunately we haven't documented that component thoroughly yet.

this does solve the use case I was looking for, however, I am wondering if that is a lot of overhead. Ex. I would have to create a broker -> trigger -> function -> channel -> subscriber...all just for what I really want function -> ksvc. It seems like (my pr) would be a tiny update in order to use a function to overlay something similar.

However, I can see clearly the benefit of having a standardized process for doing that, instead of a random concoction of different ways to skin the cat...etc.

I'll leave the issue closed and the pr closed, y'all can use it if you want.

[antoineco]

Yes I agree entirely, event-driven systems can introduce a lot of overhead. That's one of their big disadvantages.

Sometimes a good old chain of synchronous services does the job perfectly fine.