[trace-agent] adding output of 'trace-agent -info' #3164
Conversation
packaging/datadog-agent/source/agent
Outdated
| @@ -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 | |||
There was a problem hiding this comment.
can skip this file, we don't ship with the source install
packaging/osx/datadog-agent
Outdated
| @@ -111,6 +112,10 @@ case $1 in | |||
| RETURN_VALUE=$(($RETURN_VALUE || $?)) | |||
| $FORWARDERPATH info | |||
| RETURN_VALUE=$(($RETURN_VALUE || $?)) | |||
| if [ -x $TRACEAGENTPATH ]; then | |||
There was a problem hiding this comment.
can skip this file, we don't ship with osx
packaging/suse/datadog-agent.init
Outdated
| su $AGENTUSER -c "$TRACEAGENTPATH -info" | ||
| TRACEAGENT_RETURN=$? | ||
| fi | ||
| exit $(($COLLECTOR_RETURN+$DOGSTATSD_RETURN+$FORWARDER_RETURN+$TRACEAGENT_RETURN)) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
The way I see it, the exit code should reflect trace agent's status if it's enabled and discard it/ignore it otherwise.
There was a problem hiding this comment.
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
- Dropped source & OS/X patches, does not make sense yet - Still returning 0 if trace-agent fails to get its status. Indeed, some might use this script as a health check, and we don't want it to fail just because the trace-agent is not working, as it's not a base core component (yet).
ufoot
force-pushed
the
christian/traceagentstatusinfo
branch
from
f87187b to
9c0c8dc
Compare
|
I'm not convinced the trace agent status should be ignored if it's indeed enabled. I'd like to hear @olivielpeau input on this one. |
There was a problem hiding this comment.
Had one comment, other than that this looks good to me 👍
@truthbk: thanks for the ping! Since running the trace-agent is an opt-in and is configured in the datadog.conf I think it's a good thing not to use the exit code of the trace info command, for now at least.
packaging/debian/datadog-agent.init
Outdated
| @@ -176,6 +177,11 @@ case "$1" in | |||
| DOGSTATSD_RETURN=$? | |||
| su $AGENTUSER -c "$FORWARDERPATH info" | |||
| FORWARDER_RETURN=$? | |||
| TRACEAGENT_RETURN=0 | |||
| if [ -x $TRACEAGENTPATH ]; | |||
There was a problem hiding this comment.
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.
|
Also if the plan is to ship this for the |
* [trace-agent] adding output of 'trace-agent -info' on 'datadog-agent info' * [trace-agent] added support for CentOS - Dropped source & OS/X patches, does not make sense yet - Still returning 0 if trace-agent fails to get its status. Indeed, some might use this script as a health check, and we don't want it to fail just because the trace-agent is not working, as it's not a base core component (yet). * [trace-agent] not checking for trace-agent binary, should be always shipped
* master: (67 commits) [network] dont combine connection states (#3158) renames function in line with other checks [couchbase] Modified service_check_tags in couchbase.py to include user-specified tags. (#3079) [docker] fix image tag extraction Fix tests, refactor how we collect container and volume states test_docker_daemon.py: fix syntax errors Beginning work on docker_daemon tests. Add 5 opt-in checks to docker_daemon add mention of office hours (#3171) updates psycopg2 to 2.6.2 (#3170) [trace-agent] adding output of 'trace-agent -info' (#3164) [riak] Change default value in configuration example to match default value from the code move setting parameter to instance level riak security support [dns_check][ci] Bring back assertions on metrics (#3162) [powerdns_recursor] adds support for v4. (#3166) [tcp_check] Add custom tags to response_time gauge catch can't connect instead of failing on nodata found (#3127) [php-fpm] add http_host tag [dns_check] Document NXDOMAIN usage in yaml example file ...
What does this PR do?
When calling
datadog-agent info, this patch makes a subcall totrace-agent -infoto show trace agent (APM) status.Motivation
It's easier to monitor the agent status this way, provides a quick way to know what's happening.
Testing Guidelines
datadog-agent statusdisplay relevant info.Additional Notes
IMPORTANT -> must NOT ship before: DataDog/datadog-trace-agent#214