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

Add test for capture_headers in context_builder #1449

Merged

Conversation

mheiligers-godaddy
Copy link
Contributor

What does this pull request do?

PR#1405 introduced a bug where if capture_headers is false, the ContextBuilder will raise "undefined method `has_key?' for nil:NilClass"

Checklist

  • I have signed the Contributor License Agreement.
  • My code follows the style guidelines of this project (See .rubocop.yml)
  • I have rebased my changes on top of the latest main branch
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

- [ ] I have made corresponding changes to the documentation

Related issues

Copy link

cla-checker-service bot commented Mar 30, 2024

💚 CLA has been signed

@mheiligers-godaddy
Copy link
Contributor Author

❌ Author of the following commits did not sign a Contributor Agreement: 4fb1308

Please, read and sign the above mentioned agreement if you want to contribute to this project

Yes, yes I have.

@estolfo
Copy link
Contributor

estolfo commented Apr 2, 2024

Hi @mheiligers-godaddy thanks a lot for opening this PR! Are you sure you're signing the CLA with the same email address you use for this GitHub account?

@mheiligers-godaddy
Copy link
Contributor Author

Hi @mheiligers-godaddy thanks a lot for opening this PR! Are you sure you're signing the CLA with the same email address you use for this GitHub account?

Yes, but I got an email from @gtback yesterday afternoon saying the system had been updated (I didn't realize there was an additional step) so the check should pass now.

Separately, I see there's a failing spec on JRuby 9.2-13 with Rails 5.2, but it seems unrelated to my change, though I admit I haven't looked into it further than the stack trace in the action output.

@estolfo estolfo self-requested a review April 3, 2024 14:25
@reakaleek
Copy link
Member

run docs-build

@estolfo estolfo merged commit eca7cd0 into elastic:main Apr 4, 2024
56 checks passed
APM-Agents (OLD) automation moved this from In Progress to Done Apr 4, 2024
@sjohn-godaddy
Copy link

sjohn-godaddy commented Apr 4, 2024

Could we have a release with this bug fix? that would be very welcome, as our other alternative is to downgrade to 4.3 due to dependency to other gems. @estolfo

@estolfo
Copy link
Contributor

estolfo commented Apr 8, 2024

Hi @sjohn-godaddy, sure, we can do a release soon. Thanks for your patience!

@picandocodigo
Copy link
Member

@sjohn-godaddy @mheiligers-godaddy v4.7.3 was released with this fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants