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

fix:Convert custom setting value to string for startup log #1109

Merged
merged 2 commits into from
Jul 21, 2020

Conversation

marcotc
Copy link
Member

@marcotc marcotc commented Jul 14, 2020

This PR ensures that integration settings that can store arbitrary values, e.g. "integration_rack_application", don't expand into a large payload like:

#<RailsApp::Application:0x00007fde60c43bc8
 @_all_autoload_paths=
  ["./rails/app/controllers",
   "./rails/app/helpers",
   "./rails/app/jobs",
   "./rails/app/models"],
 @_all_load_paths=
  ["./rails/app/controllers",
   "./rails/app/helpers",
   "./rails/app/jobs",
   "./rails/app/models"],
 @app=
  #<Datadog::Contrib::Rack::TraceMiddleware:0x00007fbfb4dda128
   @app=
    #<ActionDispatch::HostAuthorization:0x00007fbfb4dda240
     @app=
      #<Rack::Sendfile:0x00007fbfb4dda308
       @app=
        #<ActionDispatch::Executor:0x00007fbfb4dda380
         @app=
          #<ActiveSupport::Cache::Strategy::LocalCache::Middleware:0x00007fbfb4f3e000
           @app=
            #<Rack::Runtime:0x00007fbfb4dda3d0
             @app=
              #<Rack::MethodOverride:0x00007fbfb4dda3f8
               @app=
...

We currently call to_json on our startup log hash, thus calling to_hash on each object.
Because objects can have custom to_json handlers, it is not safe to perform that call.
For a Rails application, for example, you can get a SystemStackError due to recursive object resolution during to_json.

Instead they create a less descriptive, yet safer output using #to_s: "integration_rack_application":"#RailsApp::Application:0x00007fde60c43bc8"

@marcotc marcotc added the bug Involves a bug label Jul 14, 2020
@marcotc marcotc added this to the 0.38.0 milestone Jul 14, 2020
@marcotc marcotc requested a review from a team July 14, 2020 18:50
@marcotc marcotc self-assigned this Jul 14, 2020
@marcotc marcotc force-pushed the fix/startup-log-to_s branch from a0403d1 to 2781fc4 Compare July 14, 2020 18:50
brettlangdon
brettlangdon previously approved these changes Jul 21, 2020
@marcotc marcotc merged commit 9c73f56 into master Jul 21, 2020
@marcotc marcotc deleted the fix/startup-log-to_s branch July 21, 2020 19:23
keiko713 added a commit to keiko713/dd-trace-rb that referenced this pull request Nov 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Involves a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants