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

Move dogstatsd start into .profile.d script #1

Merged
merged 1 commit into from Dec 16, 2014

Conversation

Projects
None yet
2 participants
@bjeanes
Contributor

bjeanes commented Dec 10, 2014

This should skip needing users of the buildpack to add the extra level of indirection by using a second Procfile. Tested my changes and was successfully getting metrics:

metric explorer datadog 2014-12-10 15-55-23

As a bonus, the daemon will also start when you heroku run bash # or a Rails console or anything else.

Context: DevCenter article and tweet

Move dogstatsd start into .profile.d script
This should skip needing users of the buildpack to add the extra level of
indirection by using a second Procfile

@bjeanes bjeanes referenced this pull request Dec 10, 2014

Merged

Remove extra Procfile #1

chmod +x $BUILD_DIR/run-dogstatsd.sh
mkdir -p $BUILD_DIR/.profile.d
cp $BUILDPACK_DIR/extra/run-dogstatsd.sh $BUILD_DIR/.profile.d/
chmod +x $BUILD_DIR/.profile.d/run-dogstatsd.sh

This comment has been minimized.

@bjeanes

bjeanes Dec 10, 2014

Contributor

This might not strictly be necessary. I don't remember offhand if we execute these scripts or source them, but it may likely be the latter.

@bjeanes

bjeanes Dec 10, 2014

Contributor

This might not strictly be necessary. I don't remember offhand if we execute these scripts or source them, but it may likely be the latter.

exec /app/.apt/opt/datadog-agent/embedded/bin/python /app/.apt/opt/datadog-agent/agent/dogstatsd.py
(
# Load our library path first when starting up
export LD_LIBRARY_PATH=/app/.apt/opt/datadog-agent/embedded/lib:$LD_LIBRARY_PATH

This comment has been minimized.

@miketheman

miketheman Dec 10, 2014

Owner

One concern I saw when using a profile.d was that the LD_LIBRARY_PATH variable was "leaking" to other buildpacks/processes, causing problems with other things loading properly.
When you launch this node with heroku run bash, can you paste the output of the env here to confirm that it's not being reset to other processes?

@miketheman

miketheman Dec 10, 2014

Owner

One concern I saw when using a profile.d was that the LD_LIBRARY_PATH variable was "leaking" to other buildpacks/processes, causing problems with other things loading properly.
When you launch this node with heroku run bash, can you paste the output of the env here to confirm that it's not being reset to other processes?

This comment has been minimized.

@bjeanes

bjeanes Dec 10, 2014

Contributor

See L17. It wraps the export and daemon start in a subshell () so that the env vars don't leak out.

$ heroku run -a bo-datadog-buildpack-test -- "env | grep LD_"
Running `env | grep LD_` attached to terminal... up, run.1533
Log file is unwritable: '/var/log/datadog/dogstatsd.log'
2014-12-10 23:05:28,465 | INFO | dd.dogstatsd | daemon(daemon.py:154) | Starting daemon
2014-12-10 23:05:28,466 | INFO | dd.dogstatsd | daemon(daemon.py:163) | Daemon pidfile: /tmp/dogstatsd.pid

There's some noise of the daemon starting up, but no LD_LIBRARY_PATH exported.

@bjeanes

bjeanes Dec 10, 2014

Contributor

See L17. It wraps the export and daemon start in a subshell () so that the env vars don't leak out.

$ heroku run -a bo-datadog-buildpack-test -- "env | grep LD_"
Running `env | grep LD_` attached to terminal... up, run.1533
Log file is unwritable: '/var/log/datadog/dogstatsd.log'
2014-12-10 23:05:28,465 | INFO | dd.dogstatsd | daemon(daemon.py:154) | Starting daemon
2014-12-10 23:05:28,466 | INFO | dd.dogstatsd | daemon(daemon.py:163) | Daemon pidfile: /tmp/dogstatsd.pid

There's some noise of the daemon starting up, but no LD_LIBRARY_PATH exported.

@miketheman

This comment has been minimized.

Show comment
Hide comment
@miketheman

miketheman Dec 10, 2014

Owner

This looks great! I had one question, but otherwise this will simplify the deployment for others. The README will need an update as well.

Owner

miketheman commented Dec 10, 2014

This looks great! I had one question, but otherwise this will simplify the deployment for others. The README will need an update as well.

miketheman added a commit that referenced this pull request Dec 16, 2014

Merge pull request #1 from bjeanes/master
Move dogstatsd start into .profile.d script

@miketheman miketheman merged commit 3a5a5ee into miketheman:master Dec 16, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment