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

Embedding existing jmx integrations with git submodule #639

Merged
merged 4 commits into from
Jan 23, 2019

Conversation

tylerbenson
Copy link
Contributor

@tylerbenson tylerbenson commented Jan 2, 2019

Now uses a git submodule to load the integrations from integrations-core.
Will depend on a new jmxfetch release with DataDog/jmxfetch#205 before this will work.

@tylerbenson tylerbenson added the tag: do not merge Do not merge changes label Jan 2, 2019
@@ -0,0 +1,5 @@
activemq.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please add readme file in this directory explaining its contents and where those files are coming from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Does the new readme look ok?

mar-kolya
mar-kolya previously approved these changes Jan 2, 2019
Copy link
Contributor

@mar-kolya mar-kolya left a comment

Choose a reason for hiding this comment

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

Just a minor comment

@mar-kolya
Copy link
Contributor

LGTM

@tylerbenson
Copy link
Contributor Author

Can't merge this until we get a new version of jmxfetch.

realark
realark previously requested changes Jan 4, 2019
Copy link
Contributor

@realark realark left a comment

Choose a reason for hiding this comment

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

Can we come up with a more automated way to pull these files in? Generally I'd like to be able to specify a sha/tag in integrations-core and get the matching jxm files into the build or into our repo. If we copy-paste it will be tricky to track down the integrations-core version these files originated from.

We can sync up offline and I can take you through a potential solution.

@tylerbenson
Copy link
Contributor Author

tylerbenson commented Jan 4, 2019

@realark That was fully my intention. This was just intended to demonstrate the flow after those files were brought in and to validate the changes I made to jmxfetch. Sorry if that wasn't more clear.

@realark
Copy link
Contributor

realark commented Jan 4, 2019

@tylerbenson Oh sure. I'll remove my review in that case.

@realark realark dismissed their stale review January 4, 2019 04:54

Requested changes already in-flight

Using copy/paste from integrations-core.  Will depend on a new jmxfetch release with DataDog/jmxfetch#205 before this will work.
@tylerbenson tylerbenson changed the title First pass at embedding existing jmx integrations Embedding existing jmx integrations with git submodule Jan 8, 2019
task submodulesUpdate(type:Exec) {
group 'Build Setup'
description 'Initializes and updates integrations-core git submodule'
commandLine 'git', 'submodule', 'update', '--init', 'integrations-core'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update here just means populating the directory with the repo contents, it doesn't change the commit the submodule is based off of.

@tylerbenson tylerbenson dismissed mar-kolya’s stale review January 14, 2019 19:31

new implementation provided

This includes the ability to load config from resources.
@tylerbenson tylerbenson removed the tag: do not merge Do not merge changes label Jan 23, 2019
@tylerbenson
Copy link
Contributor Author

@mar-kolya This is ready for review now that we have the proper version of jmxfetch.

@tylerbenson tylerbenson merged commit 63779c7 into master Jan 23, 2019
@tylerbenson tylerbenson deleted the tyler/jmxfetch branch January 23, 2019 23:58
@tylerbenson tylerbenson added this to the 0.23.0 milestone Feb 11, 2019
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.

3 participants