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

[heartbeat] Make monitors.d directory main way to configure monitors. #8833

Closed
andrewvc opened this issue Oct 30, 2018 · 2 comments
Closed

Comments

@andrewvc
Copy link
Contributor

andrewvc commented Oct 30, 2018

In #8023 we added automatic reloading to heartbeat. In feedback from @justinkambic and my own testing it's become apparent that it's a little confusing that there are two ways to configure monitors, inline and externally.

In this issue I propose that we make automatic reloading the default for a directory named monitors.d, and that while we continue to support inline monitor definition indefinitely, we make heartbeat work more similarly to metricbeat, where external files are used to configure monitors.

The advantages are that:

  1. We could refocus the documentation around the most powerful use case
  2. Users would not have to add the heartbeat.config.monitors line to the default YAML, it would ship configured.
  3. Users could more easily enable / disable monitors.

The disadvantages are that:

  1. Users who have simple configs that rarely change will be encouraged to split their config across multiple files.
  2. Users who want to be able to edit a config without it taking effect will need to disable the reload feature or use the less emphasized 1-file method.
@justinkambic
Copy link
Contributor

So - after giving this some more thought, it may be best to continue to use the simpler, single-file approach but still provide the necessary skeleton to use the multi-file approach. Ship with a {HEARTBEAT_PATH}/monitors.d directory, perhaps with an example file, and have a commented-out config block defined in heartbeat.yml to use that folder (these new options are absent from the heartbeat.yml file in master).

That way the user is assured that they don't need to reference the docs to simply enable the multi-file reloadable feature. It's a less-disruptive change from the current startup experience, and I think it's more likely that an advanced user more accustomed to complex configurations will make the jump up to the multi-file, auto-reload feature than a less capable user will figure out how to jump back down to the simpler, single-file solution. At the same time, it insulates against issues we may encounter with the docs.

That being said, if we want manual configuration of Heartbeat to be more accessible to advanced users then this approach may not be as sensible as I think.

@andrewvc
Copy link
Contributor Author

andrewvc commented Nov 9, 2018

As a follow-up I'm going to close this issue since we've all come to agreement not to make it the main way. I have opened #9004 which adds the monitors.d directory to default installs, and lets users choose which way they want to go.

@andrewvc andrewvc closed this as completed Nov 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants