joshelser commented on a change in pull request #3762:
URL: https://github.com/apache/hbase/pull/3762#discussion_r742108746



##########
File path: conf/hbase-env.sh
##########
@@ -143,10 +143,32 @@
 # export GREP="${GREP-grep}"
 # export SED="${SED-sed}"
 
-# Uncomment to enable trace, you can change the options to use other exporters 
such as jaeger or
-# zipkin. See 
https://github.com/open-telemetry/opentelemetry-java-instrumentation on how to
+# Tracing
+# Uncomment some combination of these lines to enable tracing. You should 
change the options to use
+# the exporters appropriate to your environment. See
+# https://github.com/open-telemetry/opentelemetry-java-instrumentation for 
details on how to
 # configure exporters and other components through system properties.
-# export HBASE_TRACE_OPTS="-Dotel.resource.attributes=service.name=HBase 
-Dotel.traces.exporter=logging -Dotel.metrics.exporter=none"
+#
+# The presence HBASE_TRACE_OPTS indicates that tracing should be enabled, and 
serves as site-wide
+# settings.
+# export HBASE_TRACE_OPTS="-Dotel.traces.exporter=none 
-Dotel.metrics.exporter=none"
+#
+# Per-process configuration variables allow for fine-grained configuration 
control.
+# export HBASE_SHELL_OPTS="${HBASE_SHELL_OPTS} ${HBASE_TRACE_OPTS} 
-Dotel.resource.attributes=service.name=hbase-shell"

Review comment:
       Is it intentional that a user has to update their hbase-env.sh to 
automatically pass through HBASE_TRACE_OPTS to the relevant commands when they 
want it?
   
   Meaning, when the user wants to turn on tracing, is there harm in setting 
`otel.resource.attributes=service.name=...` for all of these opts 
automatically, but let the user set their HBASE_TRACE_OPTS as they deem 
appropriate?

##########
File path: bin/hbase
##########
@@ -493,7 +493,9 @@ add_jdk11_deps_to_classpath() {
 }
 
 add_opentelemetry_agent() {
-  if ! agent_jar=$(find lib/trace -type f -name "opentelemetry-javaagent-*" 
2>/dev/null); then
+  if [ -e "${OPENTELEMETRY_JAVAAGENT_PATH}" ] ; then
+    agent_jar="${OPENTELEMETRY_JAVAAGENT_PATH}"
+  elif ! agent_jar=$(find "${HBASE_HOME}/lib/trace" -type f -name 
"opentelemetry-javaagent-*" 2>/dev/null); then

Review comment:
       Do you want `find -L`? I'm not sure if we have already resolved 
`HBASE_HOME` to be the "real" path by the time this function is called.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to