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

Feature Request: http.client.request.created Event in diagnostics_channel #55352

Closed
antoniomuso opened this issue Oct 10, 2024 · 2 comments · Fixed by #55586
Closed

Feature Request: http.client.request.created Event in diagnostics_channel #55352

antoniomuso opened this issue Oct 10, 2024 · 2 comments · Fixed by #55586
Labels
diagnostics_channel Issues and PRs related to diagnostics channel feature request Issues that request new features to be added to Node.js. http Issues or PRs related to the http subsystem.

Comments

@antoniomuso
Copy link

What is the problem this feature will solve?

I am developing a library to trace HTTP server requests. The goal is to track requests from initiation to completion, including server-to-server (outgoing) requests.

Using diagnostics_channel and async_hooks, I have successfully implemented most of the tracking, except for extracting the body of outgoing server-to-server requests. The challenge arises because the only channel available to trace these requests, http.client.request.start, is triggered after the request body has been written.

Example

Here’s a typical server-to-server request scenario:

import http from 'http';

const req = http.request(options, (res) => {
    // Handle the response...
});

req.on('error', (e) => {
    console.error(`problem with request: ${e.message}`);
});

req.write(postData);
req.end();
// <-- The `diagnostics_channel` callback fires after end(), once the body has already been written

I need a way to trace outgoing requests before the body is written. Currently, the callback for http.client.request.start fires only after the body has been sent, which makes it impossible to capture the body at the right time.

What is the feature you are proposing to solve the problem?

The feature I would like to suggest is to add a new diagnostics_channel event that is triggered when the request object is created, before the body is written, such as:

  • New event: http.client.request.created

This would allow developers to hook into the HTTP client request lifecycle earlier, making it possible to trace the request body and other parameters at the time of request creation.

What alternatives have you considered?

I tried different approaches to take the request body, but without success. The library should work without inspector and without adding command line options to the nodejs runtime, for that reason I discarded monkey patching using import-in-the-middle and require-in-the-middle.

Rejected Approaches

  • Monkey Patching: Using libraries like import-in-the-middle or require-in-the-middle was considered, but I want the library to work without modifying the Node.js runtime or requiring command-line options.
  • Node Inspector: Similarly, using the inspector is not an option, as the library needs to work seamlessly in production environments.

If there are other approaches or best practices that would achieve this goal without relying on runtime modifications, I am open to exploring them.

Thank you for taking the time to read and consider this request.

@antoniomuso antoniomuso added the feature request Issues that request new features to be added to Node.js. label Oct 10, 2024
@github-project-automation github-project-automation bot moved this to Awaiting Triage in Node.js feature requests Oct 10, 2024
@RedYetiDev RedYetiDev added the http Issues or PRs related to the http subsystem. label Oct 10, 2024
@RedYetiDev RedYetiDev moved this from Awaiting Triage to Triaged in Node.js feature requests Oct 10, 2024
@RedYetiDev RedYetiDev added the diagnostics_channel Issues and PRs related to diagnostics channel label Oct 10, 2024
@ravin00
Copy link

ravin00 commented Oct 11, 2024

To trace the outgoing requests you can create a custom wrapper right around the request function?

@antoniomuso
Copy link
Author

To trace the outgoing requests you can create a custom wrapper right around the request function?

While creating a custom wrapper around the request function could work for tracing outgoing requests, it requires the user to modify their code to use the wrapper, which contradicts the goal of the library. The intention of the library is to function seamlessly without requiring users to add custom code or monkey-patch their environment. Additionally, users may utilize different HTTP clients (e.g., axios, got, or custom implementations), making a request wrapper less effective or incompatible across various libraries. The solution needs to work out of the box, regardless of the HTTP client being used.

RafaelGSS pushed a commit that referenced this issue Nov 1, 2024
PR-URL: #55586
Fixes: #55352
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
Reviewed-By: theanarkh <[email protected]>
louwers pushed a commit to louwers/node that referenced this issue Nov 2, 2024
PR-URL: nodejs#55586
Fixes: nodejs#55352
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
Reviewed-By: theanarkh <[email protected]>
tpoisseau pushed a commit to tpoisseau/node that referenced this issue Nov 21, 2024
PR-URL: nodejs#55586
Fixes: nodejs#55352
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
Reviewed-By: theanarkh <[email protected]>
ruyadorno pushed a commit that referenced this issue Nov 27, 2024
PR-URL: #55586
Fixes: #55352
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
Reviewed-By: theanarkh <[email protected]>
ruyadorno pushed a commit that referenced this issue Nov 27, 2024
PR-URL: #55586
Fixes: #55352
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
Reviewed-By: theanarkh <[email protected]>
ruyadorno pushed a commit that referenced this issue Nov 27, 2024
PR-URL: #55586
Fixes: #55352
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
Reviewed-By: theanarkh <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
diagnostics_channel Issues and PRs related to diagnostics channel feature request Issues that request new features to be added to Node.js. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants