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.

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

[auto-discovery] re-pipe configurations after JMXFetch restart #3415

Merged
merged 4 commits into from
Jul 21, 2017
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
13 changes: 12 additions & 1 deletion agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
)
from daemon import AgentSupervisor, Daemon
from emitter import http_emitter
from jmxfetch import get_jmx_checks
from jmxfetch import get_jmx_checks, JMXFetch

# utils
from utils.cloud_metadata import EC2
Expand Down Expand Up @@ -103,6 +103,7 @@ def __init__(self, pidfile, autorestart, start_event=True, in_developer_mode=Fal
self.sd_backend = None
self.supervisor_proxy = None
self.sd_pipe = None
self.last_jmx_piped = None

def _handle_sigterm(self, signum, frame):
"""Handles SIGTERM and SIGINT, which gracefully stops the agent."""
Expand Down Expand Up @@ -342,6 +343,15 @@ def run(self, config=None):
try:
self.sd_backend.reload_check_configs = get_config_store(
self._agentConfig).crawl_config_template()

# JMXFetch restarts should prompt reload
try:
jmx_launch = JMXFetch._get_jmx_launchtime()
if self.last_jmx_piped and self.last_jmx_piped < jmx_launch:
self.sd_backend.reload_check_configs = True
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this jmxfetch-specific code block should be here exactly: JMXFetch restarts aren't really related to the updates in the config template store (which is what the parent block is about).

On top of that, when JMXFetch restarts, it'd be nice to reload only the jmx-related check configs instead of all the check configs.

So:

  1. Could we move this logic to outside this if block (i.e. right after self.reload_configs_flag = False)?
  2. To avoid reloading all the check configs, we could do what's done here: https://github.com/DataDog/dd-agent/blob/5.15.0/agent.py#L281-L285 (which could be out in a separate function) instead of setting self.sd_backend.reload_check_configs = True

Let me know how that sounds to you, I might have missed something.

except OSError as e:
log.debug("could not stat JMX lunch file: %s", e)

except Exception as e:
log.warn('Something went wrong while looking for config template changes: %s' % str(e))

Expand Down Expand Up @@ -454,6 +464,7 @@ def _submit_jmx_service_discovery(self, jmx_sd_configs):
# JMX will unblock when it reads on the other end.
os.write(self.sd_pipe, buffer)
os.write(self.sd_pipe, SD_CONFIG_TERM)
self.last_jmx_piped = time.time()
except Exception as e:
log.exception("unable to submit YAML via pipe: %s", e)
else:
Expand Down
12 changes: 12 additions & 0 deletions jmxfetch.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@
'list_limited_attributes': "List attributes that do match one of your instances configuration but that are not being collected because it would exceed the number of metrics that can be collected",
JMX_COLLECT_COMMAND: "Start the collection of metrics based on your current configuration and display them in the console"}

JMX_LAUNCH_FILE = 'jmx.launch'
Copy link
Member Author

Choose a reason for hiding this comment

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

maybe it should be a hidden file.

Copy link
Member

Choose a reason for hiding this comment

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

since it's in jmxfetch's temp directory I'd say a regular file is okay


LINK_TO_DOC = "See http://docs.datadoghq.com/integrations/java/ for more information"


Expand Down Expand Up @@ -336,6 +338,16 @@ def _start(self, path_to_java, java_run_opts, jmx_checks, command, reporter, too
log.exception("Couldn't launch JMXFetch")
raise

@staticmethod
def _get_jmx_launchtime():
fpath = os.path.join(get_jmx_pipe_path(), JMX_LAUNCH_FILE)
try:
_stat = os.stat(fpath)
except OSError as e:
raise e

return _stat.st_mtime

@staticmethod
def _is_jmx_check(check_config, check_name, checks_list):
init_config = check_config.get('init_config', {}) or {}
Expand Down