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

[SOL-57955] Allow for Solace message properties to be externally configurable #93

Closed
MikeR13 opened this issue Oct 12, 2021 · 12 comments · Fixed by #111
Closed

[SOL-57955] Allow for Solace message properties to be externally configurable #93

MikeR13 opened this issue Oct 12, 2021 · 12 comments · Fixed by #111
Assignees
Labels
enhancement New feature or request tracked Internally tracked by Solace's internal issue tracking system
Milestone

Comments

@MikeR13
Copy link

MikeR13 commented Oct 12, 2021

With the Solace Spring Cloud Stream Binder version 2.1.1 you had the possibility to configure a "message time to live" for all producer binding (default) or dedicated on every producer binding.

As I have seen, this is no longer available in version 3.1.0, or did I miss something?

It would be nice to have this feature back.

@Nephery
Copy link
Collaborator

Nephery commented Oct 12, 2021

You can set the message time to live by setting the solace_timeToLive header (you can use the SolaceHeaders.TIME_TO_LIVE constant) on your Spring message.

@MikeR13
Copy link
Author

MikeR13 commented Oct 12, 2021

Yes I know, but with this approach you have to "work" with messages (org.springframework.messaging.Message). But we want to work with the payload classes only

@Nephery
Copy link
Collaborator

Nephery commented Oct 12, 2021

Is there a reason why you need to implement your binding functions using only payloads?

Time to live is a property applied to the message, not on the binding. Which is why we moved it from a binding config to a message header config when we introduced Solace message headers in 3.0.0. If you want to make changes to the message, then to me, it makes sense to work with the message.

Pinging @Mrc0113 in case I'm overlooking a use case to have message properties be configurable in the binding config.

@Mrc0113
Copy link
Contributor

Mrc0113 commented Oct 12, 2021

Thanks for the tag @Nephery. I think @MikeR13 has a good point and we should consider allowing for settings like this to be configured on the binding. The reason for this would be to keep the messaging logic in the app's configuration and then your code is just functions focused on the business logic. It doesn't and shouldn't need to know how the Function is being invoked. This goes with the "write once use anywhere" mantra of Spring Cloud Function. Trigger the same functions via HTTP, Messaging, Rsocket, etc.

@MikeR13 in the meantime this isn't available in the current version so you could do a few things:

  1. You could just use the Message<?> object as @Nephery suggested in your main function
  2. You could use Function composition and handle adding your message properties in a separate function that knows about Message<?> from the one that does your business logic and only deals with the payload
  3. Another thing to consider might be allowing the consumer to decide how long they want the message to be available. This could be advantageous since some consumers might want to consume all the events no matter how old they are while others may say just throw them away after 30 seconds. You can configure this on the queue (or queue template) for each consumer. Basically tell the queue to "Respect TTL" and then you can set a "Maximum TTL (sec)" for each.

Hope that helps!

@Nephery Nephery added the enhancement New feature or request label Oct 12, 2021
@Nephery Nephery changed the title Missing com.solace.spring.cloud.stream.binder.properties.SolaceProducerProperties#msgTtl in version 3.1.0 Allow for Solace message properties to be externally configurable Oct 12, 2021
@Nephery
Copy link
Collaborator

Nephery commented Oct 12, 2021

Changed the title to more generally reflect the use case of having Solace message headers be externally configurable (e.g. via properties file, environment, etc).

My initial thoughts

We'll need to determine which headers makes sense to be externally configurable and which do not. e.g. having message expiration time be externally configurable probably doesn't make sense.

As for how these should be configured, it's worth referencing the Rabbit and Kafka binders to see how they do this (if at all), or if Spring Cloud Stream already has a native way to set default custom message headers in the external config. For the latter option, it may be worth asking Spring team if this is something that should be implemented in SCSt framework itself if the enhancement request doesn't exist yet.
@Mrc0113 would be good to know if you already have some insight on this.

@MikeR13
Copy link
Author

MikeR13 commented Oct 13, 2021

@Mrc0113 @Nephery
"the reason for this would be to keep the messaging logic in the app's configuration and then your code is just functions focused on the business logic. It doesn't and shouldn't need to know how the Function is being invoked."
That's exactly my point.

Thanks for listing the options!

@GreenRover
Copy link
Contributor

+1 Any idea what is the target release?

I would suggest following header for this feature:

  • ttl
  • priority
  • dmq eligible
  • cos
  • http content-type (i guess this is already)

@Mrc0113
Copy link
Contributor

Mrc0113 commented Oct 13, 2021

+1 Any idea what is the target release?

I would suggest following header for this feature:

  • ttl
  • priority
  • dmq eligible
  • cos
  • http content-type (i guess this is already)

Thanks for the suggestions @GreenRover. We will be scoping the next binder release soon so it shouldn't be too long until we have dates. Is this something that you would use as well?

@GreenRover
Copy link
Contributor

The thread owner are from sbb as well. There was one more team asking for the configuration based ttl option one month ago.

@PhilippeKhalife PhilippeKhalife added this to the 2.2.0 milestone Oct 19, 2021
@PhilippeKhalife
Copy link
Contributor

Will review all headers at design time.

@Nephery Nephery changed the title Allow for Solace message properties to be externally configurable [SOL-57955] Allow for Solace message properties to be externally configurable Oct 21, 2021
@Nephery Nephery added the tracked Internally tracked by Solace's internal issue tracking system label Oct 21, 2021
@Nephery Nephery self-assigned this Oct 21, 2021
@Nephery
Copy link
Collaborator

Nephery commented Nov 10, 2021

@MikeR13 @GreenRover @Mrc0113 The Spring Cloud Function maintainers have implemented a way to set default Spring message headers on outbound messages: spring-cloud/spring-cloud-function#764

So once that's released you'll be able to use SpEL to set default properties on outbound messages:

spring:
  cloud:
    function:
      definition: messageHandler
      configuration:
        messageHandler:
          output-header-mapping-expression:
            solace_applicationMessageId: "headers['solace_applicationMessageId'] ?: 'default-value'"
            solace_timeToLive: "headers['solace_timeToLive'] > 0 ? headers['solace_timeToLive'] : 5000L"

Or if you want to overwrite values:

spring:
  cloud:
    function:
      definition: messageHandler
      configuration:
        messageHandler:
          output-header-mapping-expression:
            solace_applicationMessageId: "'override-value'"
            solace_timeToLive: 5000L

@Nephery Nephery mentioned this issue Mar 2, 2022
@Nephery
Copy link
Collaborator

Nephery commented Mar 2, 2022

+1 Any idea what is the target release?

I would suggest following header for this feature:

* ttl

* priority

* dmq eligible

* cos

* http content-type (i guess this is already)

@GreenRover The binder doesn't currently support getting/setting message class of service. If you need this, can you raise a new issue so we can consider it for a future release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request tracked Internally tracked by Solace's internal issue tracking system
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants