HBASE-22120 Replace HTrace with OpenTelemetry #2901
Merged
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. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
saintstack
reviewed
Jan 30, 2021
Contributor
saintstack
left a comment
saintstack
left a comment
There was a problem hiding this comment.
Looking great.
We just remove trace from WAL subsystem?
Any pictures? Any traces you can share or is that later?
|
|
||
| if [[ -n "${HBASE_TRACE_OPTS}" ]]; then | ||
| echo "Attach opentelemetry agent to enable trace" | ||
| enable_trace |
Contributor
There was a problem hiding this comment.
This trick looks good. Where will you document it? Maybe if there was an 'hbase trace' command and if you ran it, you'd get output telling you to populate HBASE_TRACE_OPTs giving examples... perhaps that would be enough to get folks going? Would also advertise the new facility exits.
Contributor
Author
There was a problem hiding this comment.
In hbase-env.sh, uncomment the last line and modify the options. Will fill this in the ref guide.
| } | ||
| return ClientMetaTableAccessor | ||
| .getTableHRegionLocations(conn.getTable(TableName.META_TABLE_NAME), tableName); | ||
| }, getClass().getSimpleName() + ".getAllRegionLocations"); |
Contributor
There was a problem hiding this comment.
Above you just do getName but here getSimpleName. What you thinking? Is full classname needed sometimes and other times not?
Contributor
There was a problem hiding this comment.
Or annotation w/ servername doesn't make sense here?
Contributor
Author
There was a problem hiding this comment.
Should all be getSimpleName. Let me check.
| .setAttribute(TraceUtil.RPC_SERVICE_KEY, md.getService().getName()) | ||
| .setAttribute(TraceUtil.RPC_METHOD_KEY, md.getName()) | ||
| .setAttribute(TraceUtil.REMOTE_HOST_KEY, addr.getHostName()) | ||
| .setAttribute(TraceUtil.REMOTE_PORT_KEY, addr.getPort()); |
Contributor
There was a problem hiding this comment.
These annotations stay for the life of the Span? If so, nice.
Contributor
Author
In WAL, we switch the thread and also use batching. So we need to use some advanced features in otel like span links, etc. The plan is to implement as follow on issues. And for WAL, since we have our own DFS output stream implementation, we could add more detailed tracing to know the datanode we communicate with.
A bit busy these days. I've already deployed a cluster with 10 nodes and enabled tracing, but haven't gotten enough time to run PE or YCSB and verify the result. Could post some pictures about the topology the tracing later. The backend is skywalking, the UI is more beautiful than jaeger :) |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
saintstack
approved these changes
Apr 19, 2021
|
|
||
| enable_trace() { | ||
| agent_jar=$(find lib/trace -type f -name "opentelemetry-javaagent-*") | ||
| HBASE_OPTS="$HBASE_OPTS -javaagent:$agent_jar $HBASE_TRACE_OPTS" |
Contributor
There was a problem hiding this comment.
It'll just fail if two classes w/ same prefix... that is fine I think and shouldn't happen usually.
| if (loc != null) { | ||
| names.add(loc.getRegion().getRegionNameAsString()); | ||
| } | ||
| } |
… of OpenTelemetry Signed-off-by: stack <[email protected]>
Signed-off-by: Guanghao Zhang <[email protected]>
#2808) Signed-off-by: Guanghao Zhang <[email protected]>
Signed-off-by: Guanghao Zhang <[email protected]>
Signed-off-by: stack <[email protected]>
…nt side (#2857) Signed-off-by: Guanghao Zhang <[email protected]>
Signed-off-by: Guanghao Zhang <[email protected]>
Signed-off-by: Guanghao Zhang <[email protected]>
Signed-off-by: Guanghao Zhang <[email protected]>
Signed-off-by: Guanghao Zhang <[email protected]>
Signed-off-by: Guanghao Zhang <[email protected]>
Signed-off-by: Yulin Niu <[email protected]>
…xt (#3119) Signed-off-by: Viraj Jasani <[email protected]>
…ing opentelemtry to 1.0.0 (#3123) Signed-off-by: Michael Stack <[email protected]>
Signed-off-by: Michael Stack <[email protected]> Signed-off-by: Yulin Niu <[email protected]>
…OpenTelemetry (#3135) Signed-off-by: Michael Stack <[email protected]>
…ck is incorrect (#3165) Signed-off-by: meiyi <[email protected]>
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
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.
No description provided.