HADOOP-15566. Support Opentracing #1846
Open
+597
−162
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
💔 -1 overall
This message was automatically generated. |
|
HADOOP-16824 is blocking this PR. |
|
💔 -1 overall
This message was automatically generated. |
jojochuang
reviewed
Feb 18, 2020
hadoop-common-project/hadoop-common/src/main/proto/RpcHeader.proto
Outdated
Show resolved
Hide resolved
jojochuang
reviewed
Feb 18, 2020
hadoop-hdfs-project/hadoop-hdfs-client/src/main/proto/datatransfer.proto
Outdated
Show resolved
Hide resolved
jojochuang
reviewed
Feb 18, 2020
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
jojochuang
reviewed
Feb 22, 2020
There was a problem hiding this comment.
If this method is no longer needed, let's remove it entirely.
There was a problem hiding this comment.
did you forget to log variable byteString?
There was a problem hiding this comment.
missing a parameter here?
There was a problem hiding this comment.
i need to check if the concept matches but io.opentracing.Tracer offers a addReference() API to represent the casual relationship.
adamantal
reviewed
Feb 24, 2020
There was a problem hiding this comment.
I took a quick glance at this patch. I don't really have context on this, so I don't want to interfere.
I suggested to add javadoc which would describe the new classes and their intended behaviours.
Also, I see the the current TestTracing is empty. Can we try some TDD-approach / add test case to have more insight what is the goal here?
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/tracing/Span.java
Outdated
Show resolved
Hide resolved
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/tracing/TraceScope.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Could you please add javadoc here?
jojochuang
reviewed
Mar 4, 2020
There was a problem hiding this comment.
This is dangerous
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/io/ObjectInputStream.html
"Warning: Deserialization of untrusted data is inherently dangerous and should be avoided."
We need an alternative.
There was a problem hiding this comment.
Moreover, ObjectInputStream doesn't guarantee compatibility between a server and a client if they are on different version. Finally, I bet it is not optimal serialization format. We should find one that's as short as possible.
There was a problem hiding this comment.
Good find @jojochuang . I agree we should definitely use a more optimized solution for serialization/deserialization if possible.
Change-Id: Ia6adf57210161aea6dd4111adee460a8cfe87039
…rdparty.protobuf. nice. Change-Id: I104fbf4fa16f38eb0de63c62e5f2b0a3c96e1fad
Change-Id: I80a113075938defd0902753a331f2adc6289a8f5
…ceInfoProto, BaseHeaderProto/ReleaseShortCircuitAccessRequestProto/ShortCircuitShmRequestProto to DataTransferTraceInfoProto. Change-Id: I53223547601f7fc0f450c3c512728324ef67b3e8
Change-Id: I236788b52f0542d1fca5f902131a815bdfa298f7
Change-Id: I3e622e7febd8d385d5c6fff8c8572accb39c0fc9
|
Rebased onto latest trunk (44afe11): Resolved a minor import conflict in NameNode class due to the introduction of |
|
I created a branch "HADOOP-15566-OpenTracing" please raise a PR against that branch in the future. Thanks! |
I've updated this PR's base branch to |
|
💔 -1 overall
This message was automatically generated. |
This was referenced Jan 20, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replacing HTrace with OpenTracing.
mvn install -Pdist -DskipTests -e -Dmaven.javadoc.skip=true -Denforcer.skip=true -DskipShade