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 ActionPack instrumentation #starts_with? error #1489

Merged
merged 2 commits into from
Apr 29, 2021

Conversation

delner
Copy link
Contributor

@delner delner commented Apr 29, 2021

When running ActionPack standalone (without Rails), ActionController instrumentation may raise undefined method 'starts_with?' when processing requests. This bug was discovered when running the Rails test suite with Datadog tracing enabled.

This occurs because starts_with? is an ActiveSupport function, which may not be available. This pull request changes the function to start_with? which is the Ruby core library version, so it should be available outside Rails as well.

This bug was not caught earlier because we have a gap in our test coverage over this controller instrumentation. This pull request tests for this specific bug, but does not fully address that gap: something we will want to do soon, possibly accompanying a refactor of the instrumentation itself.

@delner delner added bug Involves a bug integrations Involves tracing integrations labels Apr 29, 2021
@delner delner added this to the 0.49.0 milestone Apr 29, 2021
@delner delner requested a review from marcotc April 29, 2021 19:16
@delner delner self-assigned this Apr 29, 2021
@delner delner requested a review from a team April 29, 2021 19:16
@delner delner added this to In review in Active work Apr 29, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #1489 (8a35b6e) into master (6ecfa4e) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1489      +/-   ##
==========================================
- Coverage   98.22%   98.22%   -0.01%     
==========================================
  Files         860      861       +1     
  Lines       41323    41361      +38     
==========================================
+ Hits        40589    40626      +37     
- Misses        734      735       +1     
Impacted Files Coverage Δ
...b/action_pack/action_controller/instrumentation.rb 94.28% <100.00%> (-1.43%) ⬇️
lib/ddtrace/contrib/action_pack/utils.rb 100.00% <100.00%> (ø)
...ion_pack/action_controller/instrumentation_spec.rb 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6ecfa4e...8a35b6e. Read the comment docs.

@delner delner merged commit 00c9501 into master Apr 29, 2021
Active work automation moved this from In review to Merged & awaiting release Apr 29, 2021
@delner delner deleted the fix/rails_controller_error_detection branch April 29, 2021 20:27
@ivoanjo ivoanjo moved this from Merged & awaiting release to Released in Active work Aug 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Involves a bug integrations Involves tracing integrations
Projects
Active work
  
Released
Development

Successfully merging this pull request may close these issues.

None yet

3 participants