nats-io / nats-server

High-Performance server for NATS.io, the cloud and edge native messaging system.
https://nats.io
Apache License 2.0
15.97k stars 1.41k forks source link

Include payload in term advisories #5049

Open ripienaar opened 9 months ago

ripienaar commented 9 months ago

Proposed change

Termination advisories include all the message data to help one find the message in the stream that holds the message, but:

  1. Streams can be removing messages from due to limits meaning the ability to fetch the data might be short lived
  2. It's expensive to go feetch the message to build something like a DLQ

The proposal is to include all the original message data in the term advisory - original timestamp in addition to the timestamp the event was create and most importantly the origianal message payload.

Use case

One can read these advisories and build a full DLQ that holds payload and all without aditional API integrations.

Contribution

No response

amitamit281 commented 9 months ago

Hi @ripienaar Will the payload be published on the same existing subject or on a different subject, such as "JS.EVENT.ADVISORY.CONSUMER.MSG_TERMINATED.."? This inquiry stems from our concern regarding the presence of compressed or confidential data in the stream. We currently subscribe to these advisories and forward them to Datadog and Splunk for alerts. However, we wish to refrain from sending data to Splunk and Datadog. If possible, it would be preferable to publish the payload on a separate subject. This would allow us to subscribe payload to a Dead Letter Queue (DLQ) Stream and republish the data if necessary.

ripienaar commented 9 months ago

We planned to send it to the same subject - that was the feature request.

I guess we would need to make it optional. It sending to a different subject seems like quite a big difference

ripienaar commented 9 months ago

@amitamit281 can you not remove this field in the tool you use to integrate with Splunk? We would rather not go crazy with too many options and settings so trying to keep it easy

amitamit281 commented 9 months ago

@ripienaar yes we can remove the payload. However, we also intend to store the payload in a stream. If it's published on a separate subject, it would be beneficial.

Jarema commented 9 months ago

@ripienaar For completness we should also include headers.

bruth commented 8 months ago

@ripienaar @Jarema would this be reasonable for the 2.11 timeline (seems straightforwarrd)? If so, feel free to set the milestone and assign someone.

ripienaar commented 8 months ago

@bruth no we decided to park it while we design a better DLQ solution due to comments about data leakage