Skip to content

Feature/fix zipkin spans alternate#5373

Merged
davsclaus merged 6 commits into
apache:masterfrom
samratdhillon:feature/fix-zipkin-spans-alternate
Apr 15, 2021
Merged

Feature/fix zipkin spans alternate#5373
davsclaus merged 6 commits into
apache:masterfrom
samratdhillon:feature/fix-zipkin-spans-alternate

Conversation

@samratdhillon
Copy link
Copy Markdown
Contributor

Fixing CAMEL-16509

Cleaner way of implementing copy safe properties on ExtendedExchange

Samrat Dhillon added 5 commits April 14, 2021 11:59
…el processing of multicast or recipientlist. The original ZipkinState object ends up getting shared by different exchange object copies resulting in issues during push/pop operations. Added interface to allow safe copy of properties which can provide deep object copies for safe copy operations

/**
*
* @author Samrat.Dhillon
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please remove author tags

*
* @param <T>
*
* Interface that allows safe copy of property value object when creating copy of Exchange objects. Classes
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you cleanup the javadoc of this

*/
public interface CamelCopySafeProperty<T> {

CamelCopySafeProperty<T> safeCopy();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add javadoc here, as its part of camel-api which we should have documented

* Interface that allows safe copy of property value object when creating copy of Exchange objects. Classes
* implementing this interface are responsible for creating deep copy of the property value object.
*/
public interface CamelCopySafeProperty<T> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove Camel in the name, and maybe SafeCopyProperty is a bit better name

void setDefaultConsumerCallback(AsyncCallback callback);

/**
* Method to set properties which for which deep copies can be created
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Polish javadoc, and same for method below

@Override
public <T> T getCopySafeProperty(String key, Class<T> type) {

Object value = getCopySafeProperties().get(key);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Optimize to check if there is any properties at all (ie its null) to then return null quickly

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks @davsclaus . Updated the PR based on your feedback.

@davsclaus davsclaus merged commit 98628d9 into apache:master Apr 15, 2021
@davsclaus
Copy link
Copy Markdown
Contributor

Can you work on the other tracer components to let them use safe copy too?

@samratdhillon
Copy link
Copy Markdown
Contributor Author

Can you work on the other tracer components to let them use safe copy too?

@davsclaus I will take a look at opentracing next. However is it okay if I backport this? We are currently working with 3.7.3 and a fixed zipkin implementation will be very helpful.

@davsclaus
Copy link
Copy Markdown
Contributor

ok you can do that

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.

2 participants