Skip to content

[jmx][auto-discovery] adds jmx support for auto_discovery#301

Merged
truthbk merged 5 commits intomasterfrom
jaime/jmx_ac
Jun 15, 2017
Merged

[jmx][auto-discovery] adds jmx support for auto_discovery#301
truthbk merged 5 commits intomasterfrom
jaime/jmx_ac

Conversation

@truthbk
Copy link
Copy Markdown
Member

@truthbk truthbk commented Jun 14, 2017

What does this PR do?

This PR adds support for auto-discovery of jmxfetch and, more specifically, relay the respective configuration to JMX via the current established mechanism - named pipes.

I have also snuck in a fix to the SNMP check. I will probably get it out and into its own PR, but I have to work some git magic because I already partially squashed this 😅 SNMP stuff moved to its own PR here

Motivation

Necessary feature for agent6 and auto-discovery.

Additional Notes

When we get the bandwidth, I would much rather have this implemented with gRPC if the overhead is acceptable (in terms of binary size, etc).

@truthbk truthbk changed the title [jmx][auto_discovery] adds jmx support for auto_discovery [jmx][auto-discovery] adds jmx support for auto_discovery Jun 14, 2017
Comment thread pkg-config/linux/python-2.7.pc Outdated
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think we can remove this, I added this before finding the USE_SYSTEM_PY option, then forgot about it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok for me, I'd like to rethink the entire USE_SYSTEM_PY thing so the less thing we have around, the better

Copy link
Copy Markdown
Contributor

@masci masci left a comment

Choose a reason for hiding this comment

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

A couple of nits otherwise 👍

Comment thread pkg/collector/check/check.go Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we add a very simple unit test for this?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd rename this module to jmxloader.go or loaders.go so we can accommodate more loaders in the same package if needed

truthbk added 5 commits June 15, 2017 11:45
[jmxfetch] bumping jmxfetch to 0.14.0.

[pipe][windows] fixing compilation issues.

[jmx] try to instantiate loader, if we fail to create pipe - skip.

[py] skip python if we cant load python loader.

[jmx][auto-discovery] implementing whitelist for specific jmxfetch checks.

[jmx][auto-discovery] fix output YAML indentation.

[config] include String() method.

[auto-discovery] separator includes cr.

[auto-discovery] renaming some more vars.
@masci
Copy link
Copy Markdown
Contributor

masci commented Jun 15, 2017

:shipit:

@truthbk truthbk merged commit 6046e9b into master Jun 15, 2017
@truthbk truthbk deleted the jaime/jmx_ac branch June 15, 2017 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants