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

Add monitors.d directory + config to default configuration #9004

Merged
merged 1 commit into from
Dec 21, 2018

Conversation

andrewvc
Copy link
Contributor

@andrewvc andrewvc commented Nov 9, 2018

This makes heartbeat much easier to use by default, and less confusing in terms of where/how
to create this directory.

@andrewvc
Copy link
Contributor Author

andrewvc commented Nov 9, 2018

For any reviewers out there, how can we test this in the context of apt/deb to make sure those packages add the same files to the directory layout?

@andrewkroh
Copy link
Member

For each package type you’ll need to apply a bit of customization to add your monitors.d directory to the package. Take a look at the magefile.go for filebeat/metricbeat/auditbeat for an example.

After building the packages (mage package) we execute a test that checks the contents of each package type. We have a way of checking for modules.d in each package type. Maybe you could change the existing checks to be configurable (like allow monitors.d to be changed to monitors.d via a CLI flag). See
https://github.com/elastic/beats/blob/master/dev-tools/packaging/package_test.go#L61.

Hopefully this will get you started. I’m happy to chat after you’ve taken a look at this stuff.

@andrewvc andrewvc added the Team:obs-ds-hosted-services Label for the Observability Hosted Services team label Nov 26, 2018
@ruflin ruflin added Team:obs-ds-hosted-services Label for the Observability Hosted Services team and removed Team:obs-ds-hosted-services Label for the Observability Hosted Services team labels Dec 3, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/uptime

@ruflin
Copy link
Member

ruflin commented Dec 13, 2018

@andrewvc Anything I can help with to push this PR forward?

@andrewvc
Copy link
Contributor Author

@ruflin not quite ready for review yet. I just rebased it and added support in the magefile.
I still need to write tests (and manually test it).

Also fixed heartbeat.yml to point at a relative path

@andrewvc
Copy link
Contributor Author

OK, things seem to work and package tests are added. I actually haven't manually tested them yet. Can do tomorrow.

There's a lot of opportunity to DRY things up in the package tests, but it feels premature to me for the effort. WDYT @ruflin / @andrewkroh ?

dev-tools/mage/pkg.go Outdated Show resolved Hide resolved
@ruflin
Copy link
Member

ruflin commented Dec 18, 2018

@andrewvc Code LGTM. Would be good if you could also test it locally and after merging keep an eye on the packaging tests.

In case you rebase again on master or do an additonal push, could you also make Hound happy? CI failures are not related.

@andrewvc
Copy link
Contributor Author

@ruflin I tested this on OSX/Ubuntu Server/Fedora VMs using .tar.gz/.deb/.rpm respectively .

Everything worked great. On the linux box I set things up as a service using ES as a service. All I had to do to get a working monitor was remove the .disabled from a monitor in the sample dir and customize it to my liking.

I've improved the PR somewhat adding separate sample files per protocol to monitors.d.

@andrewvc
Copy link
Contributor Author

jenkins, please retest this

looks like weird transient yum/docker issues

Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

CI failure do not seem to be related and the ones failing are "in the works" AFAIK.

heartbeat/_meta/beat.yml Outdated Show resolved Hide resolved
@andrewvc
Copy link
Contributor Author

@ruflin I've set reload: false as the default, squashed and rebased as well. LMK when we're good to merge.

CHANGELOG.asciidoc Outdated Show resolved Hide resolved
@andrewvc andrewvc force-pushed the monitord-default branch 2 times, most recently from 018bdfd to 7b72ca4 Compare December 20, 2018 23:06
This makes heartbeat much easier to use by default, and less confusing in terms of where/how
to create this directory.
Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

CI failures are not related.

@andrewvc andrewvc merged commit 2e92a04 into elastic:master Dec 21, 2018
andrewvc added a commit to andrewvc/beats that referenced this pull request Jan 4, 2019
)

This makes heartbeat much easier to use by default, and less confusing in terms of where/how
to create this directory.

(cherry picked from commit 2e92a04)
andrewvc added a commit that referenced this pull request Jan 7, 2019
…9757)

This makes heartbeat much easier to use by default, and less confusing in terms of where/how
to create this directory.

(cherry picked from commit 2e92a04)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants