Instrument HTTP.rb chained requests on version 6#1528
Merged
Conversation
In http 6 a chained request -- HTTP.follow.get(...), HTTP.headers(...).get(...), and so on -- runs through HTTP::Session rather than HTTP::Client, which never touches the instrumented Client#request. Those requests were not recorded. Prepend the request instrumentation onto HTTP::Session too, so a chained request produces a request.http_rb event like any other. http 5 has no Session; chained requests go through Client#request there.
|
✔️ All good! |
unflxw
changed the title
http 6
unflxw
changed the title
http 6
unflxw
added
the
bug
There was a problem hiding this comment.
Pull request overview
This PR closes an instrumentation gap in the HTTP.rb integration for http v6 by ensuring chained requests (e.g., HTTP.follow.get, HTTP.headers(...).get) emit the request.http_rb event, just like non-chained requests.
Changes:
- Prepend the existing request instrumentation module onto
HTTP::Session(http v6 chained request path) in addition toHTTP::Client. - Add a spec asserting redirect-following (
HTTP.follow) produces exactly onerequest.http_rbevent spanning all hops. - Add a hook spec verifying the
HTTP::Sessioninstrumentation module is installed when http v6 is present.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| spec/lib/appsignal/integrations/http_spec.rb | Adds a redirect-follow spec asserting a single request.http_rb event for HTTP.follow. |
| spec/lib/appsignal/hooks/http_spec.rb | Verifies HTTP::Session is instrumented for http v6. |
| lib/appsignal/integrations/http.rb | Documents the request-boundary and introduces context for HTTP::Session#request instrumentation. |
| lib/appsignal/hooks/http.rb | Installs instrumentation on HTTP::Session when available (http v6 chained requests). |
| .changesets/instrument-http-rb-chained-requests.md | Records the fix as a patch changeset. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This comment has been minimized.
This comment has been minimized.
tombruijn
approved these changes
Jun 30, 2026
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
|
This is a message from the daily scheduled checks. |
unflxw
added a commit
to appsignal/test-setups
that referenced
this pull request
Jul 9, 2026
Add Faraday GET/POST endpoints to rails7-sidekiq so the new automated Faraday integration is exercised (and its downstream Net::HTTP event suppressed). Add chained HTTP.rb requests -- a headers chain and a followed redirect -- which run through HTTP::Session on http 6 and were previously uninstrumented. Targets: appsignal/appsignal-ruby#1523 (Faraday) appsignal/appsignal-ruby#1528 (HTTP.rb chained)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #1515, spun as a separate PR to make reviewing easier.
While working on distributed tracing for HTTP clients we found a gap in the HTTP.rb integration that stands on its own, so it's split out here without any of the trace-context propagation (which is collector-mode only).
The HTTP.rb integration records its
request.http_rbevent by prepending ontoHTTP::Client#request. In http 6, though, a chained request --HTTP.follow.get(...),HTTP.headers(...).get(...),HTTP.timeout(...).get(...), and so on -- runs throughHTTP::Session#requestinstead, which never touchesHTTP::Client#request. As a result those requests were not instrumented at all: no event was recorded for them.This prepends the same request instrumentation onto
HTTP::Sessionas well, so a chained request produces arequest.http_rbevent like any other request. http 5 has noSession; chained requests go throughClient#requestthere, so it's unaffected.The event is still recorded at the request boundary, so a redirected request stays a single event spanning every hop -- the added spec drives
HTTP.followthrough a redirect and asserts exactly onerequest.http_rbevent, which fails on http 6 without this change.