prebid-server icon indicating copy to clipboard operation
prebid-server copied to clipboard

Add hook.Code to the ModuleInvocationContext

Open scr-oath opened this issue 1 year ago • 6 comments

[Fixes #4035] Add the hook.Code to the ModuleInvocationContext.

scr-oath avatar Nov 04 '24 21:11 scr-oath

@scr-oath - we discussed this in committee - can you please provide an example of how this addresses the issue? How is this helpful to your usecase?

bretg avatar Nov 15 '24 15:11 bretg

Well, there was some example in the issue this references, but imagine having a stage that has three groups…

  1. module 1 one fires off async things
  2. other modules do their work unrelated to and not blocking 1 and make mutations with their findings
  3. module 1 joins the async + the mutations in 2 and returns further mutations.

Because the module 1 needs to have information about the in flight request passed to "itself" in group 3, and needs to know that it's the 2nd invocation, passing the hookImplCode to the module invocation itself can be helpful to the module knowing which group it's in. While one might be able to pass information to subsequent stages in the returned ModuleContext, they would need to know that they could NOT use that technique in the BidderRequst and RawBidderResponse stages without getting panics due to the lack of locks being held on reading map entries while they're being updated by other bidder responses.

Tell me, what was the intended purpose of the hookImplCode if not passed to the hook for consideration (where is it used other than logging)? And what is the fear of adding to ModuleInvocationContext ?

scr-oath avatar Dec 13 '24 06:12 scr-oath

In my mind it was not intended for a module to be called twice in the same stage. Rather, the intention was that Module 1 could fire off it's async thing early in the workflow (e.g. raw auction request) and then utilize the results later (e.g. processed auction request)

If the module needs the results within the same stage, then it should just wait for the event, and not necessarily be split into two groups within the stage.

That said, the details of the feature was designed with Java VertX in mind by people who aren't here anymore. Perhaps there's something about Go that makes the architecture more awkward? From the issue:

if there is any I/O that can be started in one group and joined/Wait'ed in the 2nd - then the I/O time can overlap/be parallel with the CPU time of the first group.

The implication here is that Go (or Java) won't allow a module function to block on an I/O event without spinning CPU. If that's true, I've no objection to expanding the model.

bretg avatar Dec 13 '24 13:12 bretg

@scr-oath let's discuss this week.

bsardo avatar Feb 17 '25 04:02 bsardo

We'll discuss this at the next engineering subcommittee meeting.

bsardo avatar Mar 24 '25 02:03 bsardo

This is still on our radar for an upcoming engineering subcommittee meeting. We should have time to discuss it tomorrow.

bsardo avatar Apr 14 '25 18:04 bsardo