camunda / camunda-modeler

An integrated modeling solution for BPMN, DMN and Forms based on bpmn.io.
https://camunda.com/products/modeler
MIT License
1.5k stars 481 forks source link

I can publish a message from a workflow instance - modeling part #3738

Open barmac opened 1 year ago

barmac commented 1 year ago

This is a tracking issue to support https://github.com/camunda/zeebe/issues/1021 modeling wise.

What should we do?

Currently, in order to publish a message, you need to use an external worker (e.g. via a service task). The upcoming feature of Zeebe is to publish messages without external worker, i.e. immediately when token arrives at a message throw event or send task.

Why should we do it?

Required to use feature implemented in Zeebe via https://github.com/camunda/zeebe/issues/1021 Product hub epic: https://github.com/camunda/product-hub/issues/168

lzgabel commented 1 year ago

immediately when token arrives at a message throw event or send task.

Hi @barmac. We should also support message end event.

ingorichtsmeier commented 1 year ago

And take care about the payload, the correlation key and the time to live.

barmac commented 1 year ago

immediately when token arrives at a message throw event or send task.

Hi @barmac. We should also support message end event.

Correct, it's both Intermediate Message Throw Event and Message End Event. I tried to capture them under "Message Throw Event" name.

barmac commented 1 year ago

I added both message throw events explicitly to the list.

And take care about the payload, the correlation key and the time to live.

I added specific attributes of zeebe:publishMessage to the list.

@lzgabel How do we specify payload?

lzgabel commented 1 year ago

How do we specify payload?

Hi @korthout. BpmnMessageBehavior R65-R67 Currently, the engine uses local variables as the payload to send, what do you think about this?

nikku commented 1 year ago

Currently, the engine uses local variables as the payload to send, what do you think about this?

@korthout @lzgabel This means that every variable that is explicitly mapped into the event is being sent? I'd rather make message payload an explicit concern:

This would solve issues on three fronts:

barmac commented 1 year ago

I can imagine we define a message like so:


<bpmn:sendTask id="Activity_1kgyl85" messageRef="Message_1nb8aa6">
    <bpmn:extensionElements>
       <zeebe:publishMessage correlationKey="=orderId" payload="={clientName: client.name, paymentId: paymentId}" />
    </bpmn:extensionElements>
</bpmn:sendTask>

<bpmn:message id="Message_1nb8aa6" name="order_placed" />
korthout commented 1 year ago

I'd rather make message payload an explicit concern

@nikku @barmac @ingorichtsmeier @lzgabel I see no reason why we'd need a new concept for this. Local variables are a great fit and allow us to reuse an existing feature instead of building specialized properties.

Allow me to defend that stance.

This would solve issues on three fronts:

  • Make message payload explicit and decouple it from process variables
  • Make throw events simple targets for element templates
  • Keep input mappings optional. They are optional everywhere.
    • Message variables are already explicit. Only local variables are sent along with the message.
    • There is no need to decouple message and process variables. They are all variables, which is a core concept of C8.
  1. I'm not sure what you mean by simple targets. Are input mappings hard to use? If so, let's improve input mappings instead of building a new concept for this. Or let's see if we can find better ways to define local variables.
  2. Input mappings are still optional. If you want to publish a message without variables then simply don't define any input mappings.

I argue that using local variables integrates messages directly into the process model in a way that is familiar to users.

This concept is already in use Signal Throw events because using local variables fits so well, both in the expectations of the contributor as well as in the related implementation.


Also, please note that in C8 (as far as I'm aware), we use the terminology message variables and not message payloads:

By default, all message variables are merged into the process instance. - https://docs.camunda.io/docs/next/components/modeler/bpmn/message-events/#variable-mappings

the message variables as a JSON document - https://docs.camunda.io/docs/next/apis-tools/grpc/#input-publishmessagerequest

Usage: zbctl publish message [flags]

Flags: ... --variables string Specify message variables as JSON string or path to JSON file (default "{}")

/**

barmac commented 1 year ago

I am sold. The input mappings are a standard way of passing variables to an element, and there seems to be no need for an external extension. So this looks fine to me:

<bpmn:sendTask id="Activity_1kgyl85" messageRef="Message_1nb8aa6">
    <bpmn:extensionElements>
       <zeebe:ioMapping>
         <zeebe:input source="=client.name" target="clientName" />
         <zeebe:input source="=paymentId" target="paymentId" />
       </zeebe:ioMapping>
       <zeebe:publishMessage correlationKey="=orderId" />
    </bpmn:extensionElements>
</bpmn:sendTask>

<bpmn:message id="Message_1nb8aa6" name="order_placed" />
nikku commented 1 year ago

So as a user I'm learning: Whatever variables are explicitly input-mapped into a message throw event are being sent with the message.

UX wise the trouble I'm having with this is that nowhere else I'm aware of this explicit mapping is a thing. Rather implicit mapping is what the engine does, and if I remember correctly no guarantee exists that this mapping is conclusive. Hence I'm still advocating for making the variables passed explicit: https://github.com/camunda/camunda-modeler/issues/3738#issuecomment-1640302513.

Implementation wise the direction benefits us, as it is less work for us to do.

nikku commented 1 year ago

This concept is already in use Signal Throw events because using local variables fits so well, both in the expectations of the contributor as well as in the related implementation.

If this is already implemented, then we should follow suite and implement message throwing in the same way.

korthout commented 1 year ago

@barmac Please be aware that I just paused internal message publishing in Zeebe, because of a significant technical blocker.

I think this issue can be on hold as well, but we can continue to work out the details of passing variables along with the published message in the meantime.


If this is already implemented, then we should follow suite and implement message throwing in the same way.

TBH, signal throw events have been available since 8.3.0-alpha1. So we can still change this if necessary. If we decide this, we must make changes to signal broadcasting using Signal Throw Events before 8.3.0 is released.

UX wise the trouble I'm having with this is that nowhere else I'm aware of this explicit mapping is a thing

I guess the point is not the input mappings because input mappings always do the same: they produce local variables. That's the same here. The choice we made is to use the local variables to send the message. This is explicitly different from job worker tasks, where all variables visible from the task are passed to the job. But we considered that the usage pattern for the variables is different. Perhaps that's a wrong assumption.

🤔 Job workers have a way to limit which variables are passed to them (i.e. fetchVariable in the ActivateJobsRequest). The idea to have some explicit (and separate) way to clarify which visible variables are passed along to the message at least makes sense from that perspective.

cc @saig0 I'm curious about your opinion on this topic

PS: this topic would also be relevant to throwing errors with variables using Throw Error Events.

saig0 commented 1 year ago

@korthout I have no strong opinion if we should use input mappings or an explicit message variable property. Both options will work. :rocket:

I slightly prefer the explicit property, like @nikku mentioned, it is very explicit for the user what variables will be sent. Instead of an implicit rule to use the variables from the input mappings, we would have a dedicated property in the modeler for the message variables. Especially for not-so-experienced users, the dedicated property could be more intuitive (i.e. most users don't read the documentation first). Plus, we don't need to create local variables.

nikku commented 1 year ago

Just to add one more point to explicit vs. implicit mapping: In the future, as we may have execution listeners to generate additional input or consume it. You may want to pass information into a flow node that shall not be sent via message variables :arrow_right: explicit over implicit.

https://github.com/camunda/camunda-modeler/issues/3738#issuecomment-1643394596 resonates well with me.

korthout commented 1 year ago

You may want to pass information into a flow node that shall not be sent via message variables

@nikku Can you elaborate on this? I don't understand what this is about.

nikku commented 1 year ago

@korthout This is essentially a reflection of my mental model:

image


For me it is easier to distinguish passing data into an activity (explicitly or not), some additional processing going on and then executing the activity, with explicit data mapping. But that is just me looking at it :see_no_evil:

I think most important is that we align the implementation with the different kinds of execution that we have, so the user only needs to learn one mechanism. Called processes seem to operate like you propose (pass variables through input mappings). For called decisions I'm not sure based off the documentation.

If the user is able to learn "I need to establish local mappings to accomplish X" then we're fine, I think.

ingorichtsmeier commented 1 year ago

Hello everybody, if the property panel in the Modeler contains the term "payload" for all input mappings, it would be easy for the user to understand this feature.

Downside now: It could be the case that some users ask, where are the input mappings for message send tasks?

Especially if they decide to use (or already have) their own worker implementation to publish messages.

nikku commented 11 months ago

This is still pending upstream implementation.