fsalum / puppet-newrelic

Puppet module to install New Relic Server Monitoring and PHP agents
Apache License 2.0
26 stars 96 forks source link

newrelic_ini_appname does not make it into newrelic.ini (and presumably other vars) #70

Open kirkmadera opened 7 years ago

kirkmadera commented 7 years ago

The newrelic.ini template attempts to use data with local variable names, but no data is actually being passed to it.

I believe that all parameters in the php agent definition are now ignored because the ini file is created in a separate class scope of newrelic::php::newrelic_ini:

::newrelic::php::newrelic_ini { $newrelic_php_conf_dir:
  exec_path            => $newrelic_php_exec_path,
  newrelic_license_key => $newrelic_license_key,
  before               => [ File['/etc/newrelic/newrelic.cfg'], Service[$newrelic_php_service] ],
  require              => Package[$newrelic_php_package],
  notify               => Service[$newrelic_php_service],
}```

I'm pretty sure the template does not have access to these variables from here. The variables need to get passed into newrelic::php::newrelic_ini or the template needs to refer to each variable with the prefix `newrelic::agent::php` like `newrelic::agent::php::newrelic_ini_appname`
kirkmadera commented 7 years ago

Validated and am making a pull request in a few

kirkmadera commented 7 years ago

Can anyone review this pull request and merge it in? It breaks in Travis CI, but the errors displayed in the details seem like an issue with Travis CI or its config; not a code issue. This code works and I have been using this on projects since I submitted the PR.

kirkmadera commented 7 years ago

Any update on this? We'd like to go back to using a tagged version, but this needs to get merged in.

kirkmadera commented 7 years ago

For anyone else hitting this, another workaround is to set it in php settings (if using the mayflower/php)

php::settings:
  newrelic/newrelic.appname: My App Name

You can also do this in Nginx vhosts if using puppet/nginx if you need to set this per application:

nginx::nginx_vhosts:
  myvhost:
    # other code
    locations:
      # Other locations
      index_php:
        location: /index.php
        fastcgi: 127.0.0.1:9000
        index_files: []
        www_root:
        location_cfg_prepend:
          expires: "off"
          fastcgi_read_timeout: 18000
        fastcgi_param:
          SCRIPT_FILENAME: $document_root$fastcgi_script_name
          PHP_VALUE: "newrelic.appname=\"My App Name\""
grega commented 7 years ago

Can confirm I'm seeing the same issue.

Would be hugely helpful to have https://github.com/fsalum/puppet-newrelic/pull/71 merged!

stevenrombauts commented 7 years ago

I can also confirm the same issue. The proposed fix in #71 solves the issue. @fsalum Is there any chance you could hit the merge button on that PR? :)