Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[trace-agent] adding output of 'trace-agent -info' #3164

Merged
merged 3 commits into from
Feb 2, 2017
Merged

[trace-agent] adding output of 'trace-agent -info' #3164

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9dfdef4
[trace-agent] adding output of 'trace-agent -info' on 'datadog-agent …
ufoot Feb 1, 2017
9c0c8dc
[trace-agent] added support for CentOS
ufoot Feb 1, 2017
a0ae290
[trace-agent] not checking for trace-agent binary, should be always s…
ufoot Feb 2, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packaging/datadog-agent/source/agent
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ case $action in
RETURN_VALUE=$(($RETURN_VALUE || $?))
python agent/ddagent.py info
RETURN_VALUE=$(($RETURN_VALUE || $?))
if [ -x ./bin/trace-agent ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

can skip this file, we don't ship with the source install

./bin/trace-agent -info
RETURN_VALUE=$(($RETURN_VALUE || $?))
fi
exit $RETURN_VALUE
;;

Expand Down
8 changes: 7 additions & 1 deletion packaging/debian/datadog-agent.init
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ DOGSTATSDPATH="/opt/datadog-agent/agent/dogstatsd.py"
KILL_PATH="/opt/datadog-agent/embedded/bin/kill"
AGENTUSER="dd-agent"
FORWARDERPATH="/opt/datadog-agent/agent/ddagent.py"
TRACEAGENTPATH="/opt/datadog-agent/bin/trace-agent"
NAME="datadog-agent"
DESC="Datadog Agent"
SUPERVISOR_PIDFILE="/opt/datadog-agent/run/datadog-supervisord.pid"
Expand Down Expand Up @@ -176,7 +177,12 @@ case "$1" in
DOGSTATSD_RETURN=$?
su $AGENTUSER -c "$FORWARDERPATH info"
FORWARDER_RETURN=$?
exit $(($COLLECTOR_RETURN+$DOGSTATSD_RETURN+$FORWARDER_RETURN))
TRACEAGENT_RETURN=0
if [ -x $TRACEAGENTPATH ];
Copy link
Member

Choose a reason for hiding this comment

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

applies to the 2 other init files as well: I don't think we need to check that the trace agent binary exists and is executable, these init files are only used by the packaged Linux agent and it'll always ship the trace agent going forward, with correct exec permissions. Unless I'm missing something.

su $AGENTUSER -c "$TRACEAGENTPATH -info"
TRACEAGENT_RETURN=$?
fi
exit $(($COLLECTOR_RETURN+$DOGSTATSD_RETURN+$FORWARDER_RETURN+$TRACEAGENT_RETURN))
;;

status)
Expand Down
5 changes: 5 additions & 0 deletions packaging/osx/datadog-agent
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ DESC="Datadog Agent"
AGENTPATH="/opt/datadog-agent/agent/agent.py"
FORWARDERPATH="/opt/datadog-agent/agent/ddagent.py"
DOGSTATSDPATH="/opt/datadog-agent/agent/dogstatsd.py"
TRACEAGENTPATH="/opt/datadog-agent/bin/trace-agent"
AGENTCONF="/opt/datadog-agent/etc/datadog.conf"
SUPERVISOR_PIDFILE="/opt/datadog-agent/run/datadog-supervisord.pid"
SUPERVISOR_CONF_FILE="/opt/datadog-agent/etc/supervisor.conf"
Expand Down Expand Up @@ -111,6 +112,10 @@ case $1 in
RETURN_VALUE=$(($RETURN_VALUE || $?))
$FORWARDERPATH info
RETURN_VALUE=$(($RETURN_VALUE || $?))
if [ -x $TRACEAGENTPATH ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

can skip this file, we don't ship with osx

$TRACEAGENTPATH -info
RETURN_VALUE=$(($RETURN_VALUE || $?))
fi
exit $RETURN_VALUE
;;

Expand Down
8 changes: 7 additions & 1 deletion packaging/suse/datadog-agent.init
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ DOGSTATSDPATH="/opt/datadog-agent/agent/dogstatsd.py"
KILL_PATH="/opt/datadog-agent/embedded/bin/kill"
AGENTUSER="dd-agent"
FORWARDERPATH="/opt/datadog-agent/agent/ddagent.py"
TRACEAGENTPATH="/opt/datadog-agent/bin/trace-agent"
NAME="datadog-agent"
DESC="Datadog Agent"
SUPERVISOR_PIDFILE="/opt/datadog-agent/run/datadog-supervisord.pid"
Expand Down Expand Up @@ -417,7 +418,12 @@ case "$1" in
DOGSTATSD_RETURN=$?
su $AGENTUSER -c "$FORWARDERPATH info"
FORWARDER_RETURN=$?
exit $(($COLLECTOR_RETURN+$DOGSTATSD_RETURN+$FORWARDER_RETURN))
TRACEAGENT_RETURN=0
if [ -x $TRACEAGENTPATH ];
su $AGENTUSER -c "$TRACEAGENTPATH -info"
TRACEAGENT_RETURN=$?
fi
exit $(($COLLECTOR_RETURN+$DOGSTATSD_RETURN+$FORWARDER_RETURN+$TRACEAGENT_RETURN))
Copy link
Contributor

Choose a reason for hiding this comment

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

as mentioned in chat, i'd prefer if we don't include $TRACEAGENT_RETURN in the exit code. we mark it as an optional process, disabled by default. since info is sometimes used as a healthcheck - i think we should consider the agent "healthy" even when trace-agent -info exits non-zero. @olivielpeau any thoughts on this?

Copy link
Member

Choose a reason for hiding this comment

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

The way I see it, the exit code should reflect trace agent's status if it's enabled and discard it/ignore it otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes @truthbk really makes sense -> @talwai do we have a way to do that beyond parsing the conf for apm_enable=true & reading the env var DD_APM_ENABLED ?

Copy link
Contributor

Choose a reason for hiding this comment

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

do we have a way to do that beyond parsing the conf for apm_enable=true & reading the env var DD_APM_ENABLED ?

this is the only way. i agree with @truthbk 's logic in principle if this is not too messy to implement

;;

status)
Expand Down