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

The Collector™ #148

Merged
merged 3 commits into from
Feb 20, 2017
Merged

The Collector™ #148

merged 3 commits into from
Feb 20, 2017

Conversation

masci
Copy link
Contributor

@masci masci commented Feb 13, 2017

What does this PR do?

Adds a new component called Collector that wraps the common operations needed to start collecting data from checks. The Collector beside wrapping some code is responsible to keep the list of "active" check instances.

To simplify the code and the API, scheduler.Cancel and runner.StopCheck were made noops in case the check is not there.

Motivation

Having a clean and consistent API to interact with check instances, something the Service Discovery will heavily rely on.

}

// do we know this check instance?
if _, found := c.checks[id]; !found {
Copy link
Member

Choose a reason for hiding this comment

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

maybe should we not exist but just create the new check instead even if i wasn't running before ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this is tricky b/c we could but at the moment we can't easily because the Loaders (that we need to try to create the right type of check) expect a different configuration format. But yeah, we should make checks creation simpler and take this into consideration, will add a TODO for now

}

// ReloadCheck stops and restart a check with a new configuration
func (c *Collector) ReloadCheck(id check.ID, config, initConfig check.ConfigData) error {
Copy link
Member

Choose a reason for hiding this comment

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

can we extract a StopCheck(id check.ID) method from this? Service discovery will need it

Copy link
Member

@hkaj hkaj left a comment

Choose a reason for hiding this comment

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

lgtm

@masci masci merged commit 62b1f6a into master Feb 20, 2017
@masci masci deleted the massi/collector branch February 20, 2017 15:10
Kyle-Verhoog pushed a commit that referenced this pull request Jan 13, 2023
Add `ports` options to all Agent containers
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.

None yet

3 participants