ValidateAndOrderDataDependencies topologically sorts plugins by their Produces()/Consumes() declarations, but the result is only applied to the Produce extension point via OrderDataProducerPlugins (called here in runner.go). Every other hook list (preRequest, preAdmission, responseReceived, responseStreaming, admission) is populated by AddPlugins from GetAllPlugins, which iterates a map. So those hooks run in an order that is arbitrary and changes across EPP restarts (e.g. runPreRequestPlugins just walks the list).
This bit us in #1856: PredictedLatency.PreRequest read the live InFlightLoad attribute that InFlightLoadProducer.PreRequest mutates, so a training feature silently changed in meaning depending on which plugin happened to run first that boot. The fix there was to move the read into Produce, but that's a per-plugin workaround. Any future cross-plugin interaction in a non-Produce hook hits the same trap, and nothing in the API warns about it: Consumes() reads like a plugin-level ordering contract when it's actually Produce-only.
Proposal:
- Apply the already-computed topological order to all hook lists at startup (the sort exists, this is a few lines at the
OrderDataProducerPlugins call site).
- Decide per extension point what the order should mean. Producer-before-consumer is clearly right for PreRequest. For response hooks the reverse may be correct, since producers release state there.
- Document on
DataDependencies which extension points the ordering covers.
Even if 2 needs discussion, doing 1 alone makes hook order deterministic across restarts.
ValidateAndOrderDataDependenciestopologically sorts plugins by theirProduces()/Consumes()declarations, but the result is only applied to the Produce extension point viaOrderDataProducerPlugins(called here in runner.go). Every other hook list (preRequest, preAdmission, responseReceived, responseStreaming, admission) is populated byAddPluginsfromGetAllPlugins, which iterates a map. So those hooks run in an order that is arbitrary and changes across EPP restarts (e.g.runPreRequestPluginsjust walks the list).This bit us in #1856:
PredictedLatency.PreRequestread the liveInFlightLoadattribute thatInFlightLoadProducer.PreRequestmutates, so a training feature silently changed in meaning depending on which plugin happened to run first that boot. The fix there was to move the read into Produce, but that's a per-plugin workaround. Any future cross-plugin interaction in a non-Produce hook hits the same trap, and nothing in the API warns about it:Consumes()reads like a plugin-level ordering contract when it's actually Produce-only.Proposal:
OrderDataProducerPluginscall site).DataDependencieswhich extension points the ordering covers.Even if 2 needs discussion, doing 1 alone makes hook order deterministic across restarts.