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

Added a step describing how to include the new integration files into… #28

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

otisg
Copy link
Member

@otisg otisg commented Jan 4, 2019

… the agent build

@otisg otisg requested review from sivasamyk and bsmid January 4, 2019 15:54
@@ -90,3 +90,4 @@ How that is done depends on the data source type.
You can refer to this file to verify the list of metrics sent and their values.
9. Before submitting a PR for a new integration, follow the above steps to test the integration and
make sure the metrics are collected without any errors.
10. Once you are happy with your new integration please send the PR to have your integration included in the next agent release. To start using your new integration without waiting for the next agent release simply rebuild the agent (by running `build.sh` or `sudo mvn clean install dockerfile:build`) on the machine where you've added new integration YAMLs. This will include your new integration YAMLs in the new agent build which you can then install on the rest of your infrastructure.
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, few problems...

  1. "agent release" - it is actually spm-client release that includes integration, something unrelated to this repo and public agent users. We don't have releases of public agent package. So, each of these sets of users has different course of action.
  2. "simply rebuild the agent" - this will work just for public agent users, but not for SC users (who use spm-client)
  3. I think what we want to describe should also include editing a single yaml file and adding new yaml files to existing integrations. There are also cases where users will not want to publish a new integration (e.g. they may have added some SQL monitoring queries for their prod DB), so we should describe what to do in such cases.
  4. So perhaps the whole thing shouldn't be described as one of steps of "adding new integration", but separately, under some more general section or doc.

Copy link
Member Author

Choose a reason for hiding this comment

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

"agent release" - it is actually spm-client release that includes integration,

Right. I can rephrase that to make that clear.

"simply rebuild the agent" - this will work just for public agent users, but not for SC users (who use spm-client)

Right. I think if I rephrase for 1. I'll also automatically clarify this.

I think what we want to describe should also include editing a single yaml file and adding new yaml files to existing integrations.

We can indeed add info about that, too, but shouldn't that be elsewhere and not in a doc for adding new integrations? e.g. in how-to, under something like "How do I add or change metrics for an existing integration?"

There are also cases where users will not want to publish a new integration (e.g. they may have added some SQL monitoring queries for their prod DB), so we should describe what to do in such cases.

Isn't the (obvious) answer there "Don't submit the PR with anything non-generic, anything that is private or specific to you and your setup"?

separately, under some more general section or doc.

To me it makes sense to add here the info about what to do with a new integration to actually put it to use after initially preparing the integration yamls. What I think we can do is add a pointer to "how do I add a new integration" to the how-to doc, along with entries for things you mentioned under 3. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the (obvious) answer there "Don't submit the PR with anything non-generic, anything that is private or specific to you and your setup"?

That part is obvious, but people may not know how to distribute this config internally and how that will clash with future spm-client upgrades (if they use it).

WDYT?

Sounds good.

Copy link
Member Author

Choose a reason for hiding this comment

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

but people may not know how to distribute this config internally

Is the answer to that what I wrote, though - (re)build your own agent?
OR, if you are using spm-client package, but the yamls in that collectors dir (and restart the agent?)?

how that will clash with future spm-client upgrades (if they use it).

What is the answers, so I can add that?

  • If you modify the existing yamls that come with the agent/spm-client package you will have the usual situation where yum/apt-get do whatever they do when you touch any files included in the package.... and you will have to deal with that yourself.
  • If you add new yamls then you are good as long as there are no issues around integration yaml backwards compatibilities and such.

Copy link
Contributor

@bsmid bsmid Jan 7, 2019

Choose a reason for hiding this comment

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

Is the answer to that what I wrote, though - (re)build your own agent?

It isn't, at least not with our scripts since they build it with public repo integrations (and here we have something private that user doesn't commit). We'd probably have to explain that they want to modify build script to include their own specific yml configs or just manually distribute their own version of /opt/spm/spm-monitor/collectors to all their machines separately from agent installation. For users using spm-client it is the same story.

If you modify the existing yamls that come with the agent/spm-client package you will have the usual situation where yum/apt-get do whatever they do when you touch any files included in the package.... and you will have to deal with that yourself.

Yeah, if same file gets changed in public repo, user will end up with conflict which he'll have to resolve on his own. So we should suggest if changes are needed in any of existing public yml configs, they probably make most sense to be done on public repo and they should issue a PR with said change. If it is something private, users should probably create new yml file which doesn't have to be published in public repo, but then they have to take care of what we described in previous question.

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