Skip to content

Conversation

@GPrabhudas
Copy link
Contributor

This PR address issue-443

Ability to add properties to message before sending using context param.

Modified the producer interceptor methods to accept context parameter.

Copy link
Contributor

@cckellogg cckellogg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking api change so we may need to figure out if that is ok or figure out if backwards compatibility is needed.

@wolfstudy
Copy link
Member

The Logic LGTM +1, as @cckellogg said, maybe we can add a new API for this change, e.g:

BeforeSendWithContext(ctx context.Context, producer Producer, message *ProducerMessage)

@cckellogg
Copy link
Contributor

BeforeSendWithContext(ctx context.Context, producer Producer, message *ProducerMessage)

Adding a new function to the interface will still be a breaking change. Applications will have have to implement the new method and recompile. I think that's what we need to figure out. Do we need to support backward compatibility or is making a breaking api change ok for the next release.

@wolfstudy
Copy link
Member

BeforeSendWithContext(ctx context.Context, producer Producer, message *ProducerMessage)

Adding a new function to the interface will still be a breaking change. Applications will have have to implement the new method and recompile. I think that's what we need to figure out. Do we need to support backward compatibility or is making a breaking api change ok for the next release.

Yes, my mistake. It seems that adding context does have actual usage scenarios and can further improve the processing capabilities of the interceptor itself.

If we want to add this feature without breaking backward compatibility, can we try to directly expose a new interface to the user in ProducerOptions?

For exampels, in ProducerOptions:

// A chain of interceptors with context, These interceptors will be called at some points defined in ProducerInterceptorWithContext interface
InterceptorsWithContext ProducerInterceptorsWithContext

and introduce a new interface ProducerInterceptorsWithContext:

type ProducerInterceptorsWithContext interface {
	BeforeSendWithContext(ctx context.Context,producer Producer, message *ProducerMessage)

	OnSendAcknowledgement(producer Producer, message *ProducerMessage, msgID MessageID)
}

But it is not a very elegant way to achieve

Copy link
Contributor

@liangyuanpeng liangyuanpeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO,I agree with making a breaking api change ok for the next release..

@wolfstudy wolfstudy added this to the 0.9.0 milestone Feb 16, 2022
@freeznet freeznet modified the milestones: v0.9.0, v0.10.0 Jul 4, 2022
@RobertIndie RobertIndie modified the milestones: v0.10.0, v0.11.0 Mar 27, 2023
@RobertIndie RobertIndie modified the milestones: v0.11.0, v0.12.0 Jul 4, 2023
@RobertIndie RobertIndie modified the milestones: v0.12.0, v0.13.0 Jan 10, 2024
@RobertIndie RobertIndie modified the milestones: v0.13.0, v0.14.0 Jul 15, 2024
@RobertIndie RobertIndie modified the milestones: v0.14.0, v0.15.0 Oct 8, 2024
@RobertIndie RobertIndie modified the milestones: v0.15.0, v0.16.0 May 15, 2025
@RobertIndie RobertIndie modified the milestones: v0.16.0, v0.17.0 Jul 29, 2025
@RobertIndie RobertIndie removed this from the v0.17.0 milestone Oct 23, 2025
@RobertIndie RobertIndie added this to the v0.18.0 milestone Oct 23, 2025
@RobertIndie RobertIndie modified the milestones: v0.18.0, v0.19.0 Dec 1, 2025
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

Successfully merging this pull request may close these issues.

6 participants