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

Enable ntp check by default #1433

Merged
merged 3 commits into from
Mar 17, 2015
Merged

Enable ntp check by default #1433

merged 3 commits into from
Mar 17, 2015

Conversation

remh
Copy link
Contributor

@remh remh commented Mar 16, 2015

Also:

  • Add a better way to have checks enabled by default
  • Fix Stop packaging network.yaml #620
  • Ensure that we cast exception as string (otherwise pickle can fail
    when persisting the collector status)

Supersedes #1430

conorbranagan and others added 3 commits March 15, 2015 08:29
A host reporting more than 2-3 minutes off NTP time will throw off our alerting engine. We should set the default to 1 minute so when people enable the check they get the best results.
A higher ntp offset will prevent monitors to work properly and points submitted too far back in the past or the future are discarded.
Also:

* Add a better way to have checks enabled by default
* Fix #620
* Ensure that we cast exception as string (otherwise pickle can fail
when persisting the collector status)
@LeoCavaille
Copy link
Member

Looks good to me, will merge once it passed the CI.

remh added a commit that referenced this pull request Mar 17, 2015
@remh remh merged commit f542a17 into master Mar 17, 2015
@LeoCavaille LeoCavaille deleted the remh/default_checks branch March 18, 2015 17:28
@miketheman
Copy link
Contributor

Hi @remh !
(Looking at DataDog/chef-datadog#182) Curious how a file placed on disk with a similar name like conf.d/ntp.yaml with overrides would affect this new behavior - can you comment on how this should work?

@remh
Copy link
Contributor Author

remh commented Apr 8, 2015

Hey @miketheman !

If a .yaml file is present, the agent will use this one.
If a .yaml file is not present, the agent will load the .yaml.default

Logic is here:

dd-agent/config.py

Lines 785 to 807 in c7cb52d

conf_path = os.path.join(confd_path, '%s.yaml' % check_name)
# Default checks are checks that are enabled by default
# They read their config from the "[CHECKNAME].yaml.default" file
if check_name in DEFAULT_CHECKS:
default_conf_path = os.path.join(confd_path, '%s.yaml.default' % check_name)
else:
default_conf_path = None
conf_exists = False
if os.path.exists(conf_path):
conf_exists = True
elif not conf_exists and default_conf_path is not None:
if not os.path.exists(default_conf_path):
log.error("Default configuration file {0} is missing".format(default_conf_path))
continue
conf_path = default_conf_path
conf_exists = True
if conf_exists:
f = open(conf_path)

Does that make sense ?

@remh
Copy link
Contributor Author

remh commented Apr 8, 2015

So basically this PR DataDog/chef-datadog#182 shouldn't cause any trouble.

@miketheman
Copy link
Contributor

@remh Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stop packaging network.yaml
4 participants