bin/newrelic collides with newrelic-cli

[mzagaja]

Description

As a user, if I install and use both the newrelic-cli package from homebrew and have an app with the newrelic-ruby-agent, I will experience a collision where I can only use one of these tools unless I directly point to it. Given the tools differ in purpose and available commands, this is highly frustrating. Because the name of the application bin in either package is not configurable, I am specifying this as a "bug" but is maybe more of a change request.

Expected Behavior

I would expect the tools to either be identical packages that provide all functionality, or for one to be name spaced so I can specify which one to use without typing a full path.

Steps to Reproduce

1. brew install newrelic-cli
2. gem install newrelic_rpm
3. newrelic apm nrql query--format Text --query 'FROM Log SELECT err.name,err.message WHERE name=\'federated-graphql\' AND syslog.structured.data LIKE \'[containerName=ecs-federated-graphql-staging-%]\''

Unrecognized command: nrql
Usage: newrelic [ deployments | install ] [options]
use 'newrelic <command> -h' to see detailed command options

Your Environment

echo $PATH
/Users/mzagaja/.rvm/gems/ruby-3.1.2/bin /Users/mzagaja/.rvm/gems/ruby-3.1.2/bin /Users/mzagaja/.rvm/gems/ruby-3.1.2@global/bin /Users/mzagaja/.rvm/rubies/ruby-3.1.2/bin /Users/mzagaja/.rvm/bin /bin /Users/mzagaja/.local/share/nvm/v16.17.0/bin /Users/mzagaja/terraform /Users/mzagaja/Developer/infrastructure/bin /usr/local/sbin /usr/local/bin /System/Cryptexes/App/usr/bin /usr/bin /bin /usr/sbin /sbin /var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin /var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin /var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin /Library/Apple/usr/bin /usr/local/MacGPG2/bin

[workato-integration]

https://issues.newrelic.com/browse/NR-167627

[tannalynn]

I can see how this isn't desirable behavior. I think it might make the most sense for us to rename bin/newrelic to bin/newrelic_rpm to avoid this collision. Since renaming this utility would be a breaking change, it will need to wait until our next major release.

I'm going to relabel this as an enhancement and mark it for our next major release. Thanks for bringing this to our attention!

[workato-integration]

https://new-relic.atlassian.net/browse/NR-167627

[dinsley]

This one impacts us as well, and I'm happy to make these changes and submit a PR. Just wanted to check on how you handle deprecations like this on the project:

Would it make sense to leave the existing binstub with a deprecation notice on the renaming for a major release, and then follow up with the removal in a future major version?

[kaylareopelle]

That would be much appreciated, thank you! You've described our ideal deprecation process.

Somewhere around the deprecated code, could you add the comment, # TODO: MAJOR RELEASE? We do a search for this when we work on a major release to help us plan what to remove.

[dinsley]

Sounds good!

I noticed that there's another older version included in there from a previous rename:

https://github.com/newrelic/newrelic-ruby-agent/blob/dev/lib/new_relic/cli/command.rb#L63

Would that one be okay to remove in this PR, or would you like me to leave that one for y'all to manage? It looks like it's been deprecated for quite awhile 😅

[fallwith]

Hey @dinsley,

Would that one be okay to remove in this PR, or would you like me to leave that one for y'all to manage? It looks like it's been deprecated for quite awhile 😅

Despite the "major version" comment, I think that it is ok for you to go ahead and remove that one. Given what that code does, its age, and the fact that it's outside of public API territory for the agent itself, I support its removal.

If anyone ends up wanting to use the old name and the latest agent version, I'll be happy to add it back later.

Thanks again for taking a look at it!

[dinsley]

I've opened a PR here as the first step to resolve the name collision: #2307

I did have a question around the general future approach of the deployment notices with the Ruby RPM. It seems like the newrelic-cli library is being moved towards the more primary method of sending the change notices/markers going forward.

The CLI can be quite a pain to get installed/working in some environments like Heroku where a custom build pack is required. (but totally doable)

Are there any plans to extend the deployment notification portion of the commands here to the new change tracking API that is recommended here? or is the preferred route eventually to do away with having to manage the individual agent implementations of recording deployments/changes?

[dinsley]

Currently the markers sent with the Ruby agent result in this warning on the pages, so that's why I was wondering!

I think manually making a direct API request to the end-point would be easier and less overhead for us vs installing the CLI newrelic tooling on our dynos. We were going to just do that in our deployment process, but if there's interest in bringing an updated call into here I'll break it out and submit a PR. :)

[fallwith]

Thanks, @dinsley. I have created #2313 to express the need for targetting the new endpoint. We last touched the logic for signaling a deployment with #1461.

Are there any plans to extend the deployment notification portion of the commands here to the new change tracking API that is recommended here? or is the preferred route eventually to do away with having to manage the individual agent implementations of recording deployments/changes?

I have not seen such a preference formed yet, so we'll plan to update the agent itself to handle the new endpoint until we hear otherwise. We don't want anyone frustrated or confused by deprecation warnings when using the latest version of the agent.

If you'd like to add any thoughts to #2313 or submit a PR related to it, we'd absolutely welcome them.

I think manually making a direct API request to the end-point would be easier and less overhead for us vs installing the CLI newrelic tooling on our dynos. We were going to just do that in our deployment process, but if there's interest in bringing an updated call into here I'll break it out and submit a PR. :)

If there's anything we can do in documentation, code within the Ruby agent repo, or code elsewhere (Heroku buildpack, separate smaller tool, executable gist with an md5 sum, etc.), we'd love to explore whatever works best for you. Thanks as always for sharing your experiences - it really helps us better understand and cater to users.

[dinsley]

Awesome, thank you! I'll look at creating a PR for #2313 for this one as it should be pretty straight-forward. For our needs just getting the new change tracking API in place in the agent would be good enough, we don't currently have a huge need for anything else in the CLI at the moment.

There's a few pre-existing Heroku build-packs out there for the New Relic CLI tooling (I had prototyped one out awhile ago), but it would be much better to have one maintained officially, as the risk with using build-packs can be quite high if a third-party wants to do some damage.

I think it's something that could probably also be offered automatically through the Heroku add-ons API and automatically provisioned through their API (https://devcenter.heroku.com/articles/platform-api-reference#buildpack-installations), for those that are using New Relic that way. Users who have provisioned and set it up manually outside of Heroku's marketplace could just use the same build-pack then too.