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

Support custom configs #49

Merged
merged 1 commit into from Jun 25, 2018
Merged

Support custom configs #49

merged 1 commit into from Jun 25, 2018

Conversation

lucasm-iRonin
Copy link
Contributor

This simple change allows to provide additional DataDog configuration in the datadog folder in root of the application.

For testing on Heroku we can use this buildpack using https://github.com/DataDog/heroku-buildpack-datadog.git#custom-datadog URL.

@jyee
Copy link
Contributor

jyee commented Jun 21, 2018

Thanks for the PR. I think a few minor changes would be helpful:

  • Move this block up to ~line 28 right after we're setting the conf.d location. This will help keep all the config placement in the same general area of the script.
  • Update APP_DATADOG_DIR to /app/datadog/conf.d. We can then also add a /app/datadog/checks.d and duplicate the behavior so users can add custom checks.

@lucasm-iRonin
Copy link
Contributor Author

lucasm-iRonin commented Jun 21, 2018

@jyee thanks for review. I've added support for checks.d and updated README.

Could you check it once again, please?

@quinn
Copy link

quinn commented Jun 21, 2018

thanks for implementing this!

extra/datadog.sh Outdated
filename=$(basename -- "$file")
filename="${filename%.*}"
mkdir -p "$DD_CONF_DIR/conf.d/${filename}.d"
cp $file "$DD_CONF_DIR/conf.d/${filename}.d/conf.yml"
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nit, but let's keep the extension as .yaml, not .yml

extra/datadog.sh Outdated
cp $file "$DD_CONF_DIR/conf.d/${filename}.d/conf.yml"
done

for file in "$APP_DATADOG_CHECKS_DIR"/*.yaml; do
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I should have been a bit more clear. I was just asking for the change on line 30.

The checks.d dir contains python files that correspond to the yaml files and they'll also need to be added to the python path (logic around line 82). Let's keep this commit short and delete line 31 and this section. I can merge the config file part and we can work on adding logic bits in a separate PR.

Thanks again for all your work on this!

@lucasm-iRonin
Copy link
Contributor Author

@jyee I've made changes according to your suggestions.

We can add checks.d in separate PR because it hasn't been tested as I didn't need to customize them.

@jyee jyee merged commit 8d7c379 into DataDog:master Jun 25, 2018
@jyee
Copy link
Contributor

jyee commented Jun 25, 2018

Thanks again!

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.

None yet

4 participants