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 hook to access response in instrumentation-undici #2265

Closed
mydea opened this issue Jun 7, 2024 · 4 comments · Fixed by #2356
Closed

Add hook to access response in instrumentation-undici #2265

mydea opened this issue Jun 7, 2024 · 4 comments · Fixed by #2356

Comments

@mydea
Copy link
Contributor

mydea commented Jun 7, 2024

It would be great to have a hook that allows us access to request & response in instrumentation-undici.

Would you be open to add support for this? If so, I could work on a PR adding this. I am thinking about something like this which exists in instrumentation-http:

applyCustomAttributesOnSpan: (
  span,
  request,
  response,
) : void

Alternatively, if we could make certain things protected (e.g. subscribeToChannel) we could also extend the instrumentation class and add such things ourselves. If that's preferable I could also PR this?

@timfish
Copy link
Contributor

timfish commented Jul 29, 2024

It doesn't look like it would be that easy to add a applyCustomAttributesOnSpan hook that's called as late in the http instrumentation.

Using the undici:request:headers diagnostic channel event, we could add:

{
  responseHook?: (span: Span, request: UndiciRequest, response: UndiciResponse): void;
}

@mydea
Copy link
Contributor Author

mydea commented Jul 29, 2024

yeah, this is basically what I would have thought of :) that would be great!

@cpatti97100
Copy link

would this request be related to #2179?

@mydea
Copy link
Contributor Author

mydea commented Jul 29, 2024

would this request be related to #2179?

Probably, it's basically a generic hook that gives access to both request and response at the same time, which can be used for more specialised needs.

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

Successfully merging a pull request may close this issue.

3 participants