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 lib/bootstrap.rb #2635

Merged
merged 4 commits into from
May 23, 2024

Conversation

fallwith
Copy link
Contributor

Introduce lib/bootstrap.rb to offer bootstrapping functionality for those users who wish to introduce the New Relic agent to a Ruby application without altering the app's Gemfile manifest.

Testing instructions:

  1. Clone the feature branch

  2. Create a brand new Rails app:

rails new railsapp
  1. Use RUBYOPT to point to the bootstrap file (NOTE that the .rb extension is dropped here) and boot the Rails app:
RUBYOPT='-r /path/to/clone/lib/bootstrap' railsapp/bin/rails server
  1. Now examine railsapp/log to confirm that it contains a newrelic_agent.log file with indications that Rails library instrumentation took place.

Introduce `lib/bootstrap.rb` to offer bootstrapping functionality for
those users who wish to introduce the New Relic agent to a Ruby
application without altering the app's `Gemfile` manifest.

Testing instructions:

1. Clone the feature branch

2. Create a brand new Rails app:
  ```shell
  rails new railsapp
  ```

3. Use RUBYOPT to point to the bootstrap file (NOTE that the .rb
   extension is dropped here) and boot the Rails app:
   ```shell
   RUBYOPT='-r /path/to/clone/lib/bootstrap' railsapp/bin/rails server
   ```

4. Now examine `railsapp/log` to confirm that it contains a
   `newrelic_agent.log` file with indications that Rails library
   instrumentation took place.
Copy link
Contributor

@kaylareopelle kaylareopelle left a comment

Choose a reason for hiding this comment

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

👏 Awesome work! Especially with figuring out how to test this tricky bit of code.

lib/bootstrap.rb Outdated Show resolved Hide resolved
lib/bootstrap.rb Outdated Show resolved Hide resolved
lib/bootstrap.rb Outdated Show resolved Hide resolved
lib/bootstrap.rb Outdated
Comment on lines 18 to 35
# Instructions:
# - First, make sure the New Relic Ruby agent exists on disk. For these
# instructions, we'll assume the agent exists at `/newrelic`.
# - The agent can be downloaded as the "newrelic_rpm" gem from RubyGems.org
# and unpacked with "gem unpack"
# - The agent can be cloned from the New Relic public GitHub repo:
# https://github.com/newrelic/newrelic-ruby-agent
# - Next, use the "RUBYOPT" environment variable to require ("-r") this
# file (note that the ".rb" extension is dropped):
# ```
# export RUBYOPT="-r /newrelic/lib/bootstrap"
# ```
# - Launch an existing Ruby app as usual. For a Ruby on Rails app, this might
# involve running `bin/rails server`.
# - In the Ruby app's directory, look for and inspect
# `log/newrelic_agent.log`. If this file exists and there are no "WARN" or
# "ERROR" entries within it, then the agent was successfully introduced to
# the Ruby application.
Copy link
Contributor

Choose a reason for hiding this comment

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

Love these instructions. Does someone also need to pass a license key? If so, should that be called out here?

Copy link
Contributor

Choose a reason for hiding this comment

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

At some point a license key and some required configs are needed... but I'm not sure we want to encourage independent use of introducing the agent in this way quite yet. We actually talked about gating this behind an env var.

But on the other hand, these instructions get you 90% of the way towards a working agent, so might as well include the final steps.

I think I'd like to add both a disclaimer: NOTE: introducing the agent into your application via bootstrap is in beta., as well as the final steps for a working agent (license key and config file).

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

that sounds great! on the beta line, maybe append something like "use at your own risk"

i'm not sure if you necessarily need a config file to make the agent work since almost everything can be set by env vars. i think it will just fall back to defaults, but i'm not certain.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh you're totally right. You'll still get a WARN about a missing config, but things seem to work

Copy link
Contributor

Choose a reason for hiding this comment

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

Comment on lines +66 to +68
def self.check_for_require
warn_and_exit "#{__FILE__} is meant to be required, not invoked directly" if $PROGRAM_NAME == __FILE__
end
Copy link
Contributor

Choose a reason for hiding this comment

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

nice!

Copy link
Contributor

@tannalynn tannalynn left a comment

Choose a reason for hiding this comment

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

Great instructions and great work on such a complex issue!

hannahramadan and others added 2 commits May 21, 2024 16:07
Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com>
Copy link

SimpleCov Report

Coverage Threshold
Line 93.74% 93%
Branch 71.36% 71%

Copy link
Contributor

@kaylareopelle kaylareopelle left a comment

Choose a reason for hiding this comment

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

Thank you!

@hannahramadan hannahramadan merged commit 88dc808 into dev May 23, 2024
32 checks passed
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

5 participants