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

Stack level too deep errors on 7.0.0+ #731

Open
tannalynn opened this issue Jul 21, 2021 · 8 comments
Open

Stack level too deep errors on 7.0.0+ #731

tannalynn opened this issue Jul 21, 2021 · 8 comments

Comments

@tannalynn
Copy link
Contributor

tannalynn commented Jul 21, 2021

If you are experiencing a stack level too deep error after upgrading to 7.0+ of the ruby agent, check out our troubleshooting guide on resolving the issue in your application.

Report a New Conflict

The ruby agent is able to automatically install compatible instrumentation when certain gems are detected in the application that are known to cause a conflict with the agents use of Module#prepend.

To report a conflict to be added to the agent detection when automatically installing instrumentation, please provide the following after configuring the agent by following the troubleshooting guide

  1. which agent configuration option was used to install the compatible instrumentation (Ex. instrumentation.net_http: prepend)
  2. the gem (and version) that is conflicting with the agent. This would be the non-agent location in the stack trace that is part of the recursion. (Ex. /Users/user/.rvm/gems/ruby-2.6.5/gems/airbrake-10.0.1/lib/airbrake/rails/net_http.rb:11 or Airbrake v10.0.1)

Gems Known to Cause Prepend Conflicts (auto-detected in latest release v7.2.0)

Net::HTTP

  • Airbrake < 10.0.2
  • ScoutApm
  • Rack::MiniProfiler

Redis

  • PrometheusExporter

Resque

  • Airbrake < 11.0.3
@tannalynn tannalynn pinned this issue Jul 22, 2021
@tannalynn
Copy link
Contributor Author

Prepend conflict between httpclient instrumentation and Faraday v1.5.0.
instrumentation.httpclient: chain resolves the issue
No complete stack trace provided

Reported in #714

@oboxodo
Copy link
Contributor

oboxodo commented Jul 16, 2022

We upgraded from newrelic_rpm 6.x to 8.8.0. I had to update our newrelic.yml config file. We also use scout_apm gem which still uses chain method for instrumenting.

First time I got the Stack level too deep error regarding net_http. That got solved adding instrumentation.net_http: chain to the config file. That said, according to your code, scout should be automatically detected as conflicting with prepend. For some reason, that didn't work.

But then it failed again, this time because of redis implementation. I added instrumentation.redis: chain to the config file, but it kept trying to use prepend and showing "Installing New Relic supported Redis instrumentation using Prepend". Then I tried overriding it setting NEW_RELIC_INSTRUMENTATION_REDIS=chain and that did work. So in this case, I don't know why the env var works but the yaml config doesn't.

@fallwith
Copy link
Contributor

Hi @oboxodo,

Thanks very much for this information. I'm very glad to hear that you were able to get things working, but sorry to hear you ran into a couple of issues.

New Relic and Scout

As for the issue with our code auto-detecting Scout and then using chain instead of prepend, that code will only work if Scout has already been loaded ("required" in a Ruby context) before the New Relic code is loaded. Typically to ensure that the Scout code is loaded before the New Relic code, you would make sure New Relic comes second in Gemfile:

# Gemfile - put New Relic last
gem 'scout_apm'
gem 'newrelic_rpm'

If it is not possible to place newrelic_rpm after scout_apm in Gemfile, then there are 2 other options:

  • Manually set any desired instrumentation.* config values to chain in the config file (as you have done)
  • Tell Bundler not to load the New Relic code yet via gem 'newrelic_rpm', require: false and then manually require the gem in an initializer within the monitored application.

If you feel that the New Relic code is definitely getting loaded after the Scout code and that our code for auto-detecting may have a bug, please let us know and we'll investigate farther.

Redis Instrumentation

In order to configure the New Relic agent to use "chain" instead of "prepend", the approach is to put the following in the configuration file:

instrumentation.<INSTRUMENTATION NAME>: chain

With net/http you were able to do this with instrumentation.net_http: chain. This is because lib/new_relic/agent/instrumentation/net_http.rb uses :net_http for its named value.

With Redis, you will need to do instrumentation.redis_instrumentation: chain. This is because lib/new_relic/agent/instrumentation/redis.rb uses :redis_instrumentation for its named value.

I completely understand how "redis" would make more sense than "redis_instrumentation", but at this point we cannot rename it without breaking existing clients that use the current name.

@oboxodo
Copy link
Contributor

oboxodo commented Jul 19, 2022

Hi @fallwith, Thank you very much for your thorough reply.

... Typically to ensure that the Scout code is loaded before the New Relic code, you would make sure New Relic comes second in Gemfile.

You were right about the gems order. I switched them so that scout loads first and then newrelic, and that allowed me to use the default auto value for instrumentation.net_http.

...you will need to do instrumentation.redis_instrumentation: chain...

This didn't work, though. :-/

I made sure to replace instrumentation.redis: chain with instrumentation.redis_instrumentation: chain have that in the newrelic.yml, then removed the NEW_RELIC_INSTRUMENTATION_REDIS=chain env var I had... and this is the result:

image

That being said, as you can see switching the loading order of scout and newrelic seems to remove the need of using the chain patching method on redis, because this time preload worked and didn't throw an error. So now I don't need to change the redis loading default, but... I still think you guys have something weird going on with the redis-related code.

I'm happy to test any other ideas you might suggest to help you debug this.

@fallwith
Copy link
Contributor

Thank you, @oboxodo! I have created #1267 to track the investigation into why the configuration file based approach to setting redis to use "chain" is not working.

@fallwith
Copy link
Contributor

Hi @oboxodo. Two things:

  1. I was incorrect earlier when I stated that the configuration parameter should be instrumentation.redis_instrumentation. The correct parameter is instrumentation.redis, which you originally had. I obtained the "redis_instrumentation" value from the named :redis_instrumentation line in lib/new_relic/agent/instrumentation/redis.rb, however that the same file also has a configure_with :redis line that I missed earlier that permits the use of the shorter "redis" string.

  2. I have been unable to reproduce the problem so far. To try to eliminate as many variables as possible, I put together a minimal set of instructions and Ruby code in a gist.

If you are able to help test this issue by following the instructions in the gist, that would be super helpful. If the gist fails for you, then there must be something different between your environment and mine. If the gist succeeds for you, then there must be something different between the gist's Ruby code and New Relic configuration and those of your application.

oboxodo added a commit to oboxodo/newrelic-ruby-agent that referenced this issue Jul 22, 2022
This is so that uncommenting any of these options and setting a value will be correctly included in the `common: &default_settings` block. I discovered this while investigating newrelic#731 (comment).
@oboxodo
Copy link
Contributor

oboxodo commented Jul 22, 2022

@fallwith ok, I found the "bug", and here's my proposed fix: #1276

The problem is that the template newrelic.yml had no indentation on the default values (since this commit), and that causes that my (unindented) instrumentation.redis: chain gets ignored. Because it's not part of the &default_settings. So, it's a yaml syntax issue, caused by a bad config template.

Once I indented the setting adding two spaces to it, it worked as expected.

I noticed the problem when I saw these two lines in your gist.
Thanks!

@workato-integration
Copy link

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

No branches or pull requests

3 participants