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

Ensure that the context from incoming request gets to outgoing requests #3962

Open
scr-oath opened this issue Oct 9, 2024 · 2 comments
Open

Comments

@scr-oath
Copy link

scr-oath commented Oct 9, 2024

As a publisher / PBS host, we would like to add OpenTelemetry metrics on all I/O, and open telemetry needs the context to carry the trace/span/baggage information.

At the current time of writing, the hook executor does not pass a context, which is a descendant of the incoming http.Request.Context() and, therefore, the tracing is not connected.

Furthermore (and this may be broken out to a separate issue, which should be linked) the existing timeout mechanism when calling hooks is not tied to the incoming request no longer being valid (if its context is canceled by the http.Server, outgoing requests still continue).

The Proposal is to add a context field for hookexecution.NewHookExecutor to take either an extra context.Context param or, via Functional Options, allow the context to be set by the various endpoints as soon as the request is first seen (from its request.Context()).

By doing this, any middleware (such as open telemetry, or even other techniques such as adding a global per-request object for multiple hooks to share) can add values to the context before calling the httprouter.Handle and those values be interrogated through the context.

@bsardo
Copy link
Collaborator

bsardo commented Nov 1, 2024

Discussed with committee confirming this is an issue. We should pass the auction context through to the module infrastructure. Blockthrough has added the context on the interface and suggested that approach. Java has done something similar.

@bsardo bsardo moved this from Triage to Ready for Dev in Prebid Server Prioritization Nov 1, 2024
@bsardo bsardo changed the title [Placeholder from PMC] Ensure that the context from incoming request gets to outgoing requests Ensure that the context from incoming request gets to outgoing requests Nov 1, 2024
@scr-oath
Copy link
Author

scr-oath commented Nov 4, 2024

Context comes from the incoming request.Context() when you use this, you also notice cancel from clients that disconnect (such as navigating away), and, when using open telemetry, traces of outgoing calls can be part of the incoming trace. See #4007 for fix

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

No branches or pull requests

2 participants