cdbg_extra_class_path arguments are invalid for Jar based deployments about openjdk-runtime HOT 16 CLOSED

I believe CDB was broken for other reasons and should be working now.
@joaoandremartins, can you verify that CDB is working?
Also, although this error should be harmless, the --cdbg_extra_class_path parameter is only needed for the jetty runtime. So we should probably move it to be only provided in jetty.

from openjdk-runtime.

I verified that the Cloud Debugger still works for a JAR app deployed to App Engine Flexible, despite the logged exceptions at /var/log/app_engine/app/cdbg_java_agent.INFO.

The --cdbg_extra_class_path flag is being added from an external file:

DBG_AGENT="$( RUNTIME_DIR=$JETTY_BASE /opt/cdbg/format-env-appengine-vm.sh )"

ADD https://storage.googleapis.com/cloud-debugger/appengine-java/current/cdbg_java_agent.tar.gz /opt/cdbg/

It doesn't seem like we can do away this package completely. To turn the flag off for anything but the Jetty image, /opt/cdbg/format-env-appengine-vm.sh should be changed. e.g.,

APP_WEB_INF_DIR="${RUNTIME_DIR}/webapps/root/WEB-INF"

ARGS="-agentpath:${CDBG_ROOT}/cdbg_java_agent.so="
ARGS+="--log_dir=/var/log/app_engine"
ARGS+=",--logtostderr=false"
if [ "$GAE_IMAGE_NAME" = "jetty" ]; then
  ARGS+=",--cdbg_extra_class_path=${APP_WEB_INF_DIR}/classes:${APP_WEB_INF_DIR}/lib"
fi

On the openjdk image, $JETTY_BASE isn't set, therefore $RUNTIME_DIR isn't set, therefore we end up with a /webapps/root/WEB-INF/classes:/webapps/root/WEB-INF/lib classpath, which doesn't exist, hence the exception.

from openjdk-runtime.

I filed internal issue #37908366 with the CDB team, so they add the suggested fix to format-env-appengine-vm.sh.

@aslo can you please take a look, since Mike is away this week?

from openjdk-runtime.

One concern with this solution - it doesn't account for other web server runtimes (including tomcat, which is coming soon). Also, it looks like the $APP_WEB_INF_DIR variable is set to a jetty-specific path.

Maybe it would be better to instead for the runtime to set $APP_WEB_INF_DIR. Then the cloud debug script could just do something like:

if [ ! -z "$APP_WEB_INF_DIR" ]; then
  ARGS+=",--cdbg_extra_class_path=${APP_WEB_INF_DIR}/classes:${APP_WEB_INF_DIR}/lib"
fi

What do you think?

from openjdk-runtime.

I think you meant if [ ! -z "${RUNTIME_DIR}" ]; then, but yes, I agree your solution is more extensible.

from openjdk-runtime.

No, I'm saying the $APP_WEB_INF_DIR should be set by the runtime, since that's specific to the web server (it will be a different path in jetty vs tomcat). Then this script can just check for that variable's presence.

from openjdk-runtime.

$APP_WEB_INF_DIR is never empty because of the way it's assigned its value.

APP_WEB_INF_DIR="${RUNTIME_DIR}/webapps/root/WEB-INF"

from openjdk-runtime.

The remote /opt/cdbg/format-env-appengine-vm.sh file now contains the suggested fix.

@aslo mentioned an even better fix, which is for each runtime to specify $APP_WEB_INF_DIR, instead of the openjdk image as it's currently happening. I think it's easier to piggyback this change when other runtimes are launched, but I can be convinced otherwise.

from openjdk-runtime.

We still have the problem of the configuration being jetty-specific.

DBG_AGENT="$( RUNTIME_DIR=$JETTY_BASE /opt/cdbg/format-env-appengine-vm.sh )"

@joaoandremartins Maybe change it to:

DBG_AGENT="$( RUNTIME_DIR=$RUNTIME_DIR /opt/cdbg/format-env-appengine-vm.sh )"

and then define RUNTIME_DIR = $JETTY_BASE in the Jetty runtime.

from openjdk-runtime.

The openjdk-runtime could just only define RUNTIME_DIR to a neutral default value if it is not already defined..

The jetty-runtime could then drop in a /setup-env.d/19-debug-jetty-env.bash file that sets the RUNTIME_DIR to a jetty specific value and other extended images could do the same.

It perhaps would be better to have a name like DBG_WEBAPP_DIR and have the 20-debug-env.bash script convert to whatever the format-env-appengine-vm.sh needs. This decouples multiple env scripts from the current impl of cdbg.

from openjdk-runtime.

The format-env-appengine-vm.sh file looks like this now:

if [[ -n "${CDBG_DISABLE}" ]]; then
  >&2 echo "CDBG_DISABLE is set, Java Cloud Debugger will not be loaded"
  exit
fi

APP_WEB_INF_DIR="${RUNTIME_DIR}/webapps/root/WEB-INF"
CDBG_ROOT="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"

ARGS="-agentpath:${CDBG_ROOT}/cdbg_java_agent.so="
ARGS+="--log_dir=/var/log/app_engine"
ARGS+=",--alsologtostderr=true"

# When using Jetty/Tomcat images, the debugger should also read the
# WEB-INF/classes and WEB-INF/lib directories. When using the OpenJDK
# image (which deploys a JAR), this is not necessary.
if [[ ! -z "${RUNTIME_DIR}" ]]; then
  ARGS+=",--cdbg_extra_class_path=${APP_WEB_INF_DIR}/classes:${APP_WEB_INF_DIR}/lib"
fi

echo "${ARGS}"

So, I think it's best to just keep RUNTIME_DIR undefined in openjdk, and set it where it's needed.

Ultimately, each runtime should just set APP_WEB_INF_DIR, and format-env-appengine-vm.sh would be changed to not use RUNTIME_DIR at all.
@joaoandremartins WDYT?

from openjdk-runtime.

APP_WEB_INF_DIR is a better name than RUNTIME_DIR, but it would still be nice to have CDBG in the name so other images know why they are setting it.

Note also that in the code above, the APP_WEB_INF_DIR is set from RUNTIME_DIR before it is checked for non null status.

from openjdk-runtime.

How about this for format-env-appengine-vm.sh?

if [[ -n "${CDBG_DISABLE}" ]]; then
  >&2 echo "CDBG_DISABLE is set, Java Cloud Debugger will not be loaded"
  exit
fi

CDBG_ROOT="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"

ARGS="-agentpath:${CDBG_ROOT}/cdbg_java_agent.so="
ARGS+="--log_dir=/var/log/app_engine"
ARGS+=",--alsologtostderr=true"

# When using Jetty/Tomcat images, the debugger should also read the
# WEB-INF/classes and WEB-INF/lib directories. When using the OpenJDK
# image (which deploys a JAR), this is not necessary.
if [[ ! -z "${CDBG_APP_WEB_INF_DIR}" ]]; then
  ARGS+=",--cdbg_extra_class_path=${CDBG_APP_WEB_INF_DIR}/classes:${CDBG_APP_WEB_INF_DIR}/lib"
fi

echo "${ARGS}"

from openjdk-runtime.

@meltsufin that solution works, as long as ${CDBG_APP_WEB_INF_DIR} isn't set to anything if ${RUNTIME_DIR} is empty.
I'll try to get started on this today, after releasing the new Jetty image.

from openjdk-runtime.

Actually, I think we still need to support RUNTIME_DIR for backwards compatibility:

if [[ -n "${CDBG_DISABLE}" ]]; then
  >&2 echo "CDBG_DISABLE is set, Java Cloud Debugger will not be loaded"
  exit
fi

CDBG_ROOT="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"

ARGS="-agentpath:${CDBG_ROOT}/cdbg_java_agent.so="
ARGS+="--log_dir=/var/log/app_engine"
ARGS+=",--alsologtostderr=true"

# When using Jetty/Tomcat images, the debugger should also read the
# WEB-INF/classes and WEB-INF/lib directories. When using the OpenJDK
# image (which deploys a JAR), this is not necessary.
if [[ -n "${RUNTIME_DIR}" && -z "${CDBG_APP_WEB_INF_DIR}"]]; then
  CDBG_APP_WEB_INF_DIR="${RUNTIME_DIR}/webapps/root/WEB-INF"
fi
if [[ -n "${CDBG_APP_WEB_INF_DIR}" ]]; then
  ARGS+=",--cdbg_extra_class_path=${CDBG_APP_WEB_INF_DIR}/classes:${CDBG_APP_WEB_INF_DIR}/lib"
fi

echo "${ARGS}"

from openjdk-runtime.

@meltsufin LGTM

from openjdk-runtime.