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

Added Support and Handling For Generic Headers #931

Merged

Conversation

shrinandthakkar
Copy link
Collaborator

Summary

BrooklinEnvelope supports transporting the source header data to the destination, but is currently bounded to Kafka's header type. This change replaces that with generic header to let any other publisher-subscriber transfer events & headers via brooklin.


Testing

Added a unit test comparing the source and destination event's headers.

@@ -112,7 +112,7 @@ private ProducerRecord<byte[], byte[]> convertToProducerRecord(String topicName,
Headers headers = null;
if (event instanceof BrooklinEnvelope) {
BrooklinEnvelope envelope = (BrooklinEnvelope) event;
headers = envelope.getHeaders();
headers = envelope.getHeaders() instanceof Headers ? (Headers) envelope.getHeaders() : null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we expect a non Kafka Headers object here? If no, is it worth throwing an exception or logging an error here?

Copy link
Collaborator Author

@shrinandthakkar shrinandthakkar Apr 11, 2023

Choose a reason for hiding this comment

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

logging would be costly as it would do it for every record, I instead added an exception path which will resist in moving further if something other than kafka type is encountered.

@jzakaryan jzakaryan self-requested a review April 12, 2023 16:47
@shrinandthakkar shrinandthakkar merged commit a00a29f into linkedin:master Apr 12, 2023
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.

3 participants