feat(signer): BYOC per-capability pricing + real capability/model usage labels#3972
Open
seanhanca wants to merge 18 commits into
Open
feat(signer): BYOC per-capability pricing + real capability/model usage labels#3972seanhanca wants to merge 18 commits into
seanhanca wants to merge 18 commits into
Conversation
…3946) - Introduced a new function `LiveAIDisableRTMPOutput` to determine if RTMP egress should be skipped based on the environment variable `LIVE_AI_DISABLE_RTMP_OUTPUT`. - Updated the RTMP output collection logic in `setupStream` and `StartLiveVideo` methods to conditionally append outputs based on the new function. - Improved logging to indicate whether RTMP output is disabled, providing better visibility into the streaming configuration. These changes optimize the handling of RTMP outputs, particularly for WHEP scenarios that utilize an in-memory ring buffer instead.
Record the security fix from a6c4b1e in CHANGELOG_PENDING.md. The merge commit ("Merge commit from fork") did not document the user-facing change: validateTicketParams returned early on ExpirationBlock == 0, skipping the economic caps; it now rejects such tickets.
Bump VERSION to 0.8.11 and document the PRs merged since v0.8.10 in the changelog.
…ng tests - Implemented the ModelIDForCapability method in the Capabilities struct to return the constrained model ID for a given capability, ensuring deterministic results when multiple models are present. - Added unit tests for ModelIDForCapability to cover various scenarios, including nil receivers, no constraints, single constrained models, and handling of ambiguous model cases. - Updated the remote signer to include the model ID in the event payload for LiveVideoToVideo capabilities.
…al resolution - Modified the test for ModelIDForCapability to reflect that multiple models resolve to the lexicographically first ID instead of returning an empty string. - Adjusted the expected output in the test case to return "comfyui" for the Capability_LiveVideoToVideo capability.
- Updated the payment processing logic to include an optional `AuthID` in the `RemotePaymentState` and `authResponse` structures. - Modified the `doPaymentWithStateAndAuthID` function to accept and set the `AuthID` from the request header or callback response. - Added tests to verify the correct handling of `AuthID` in various scenarios, including fallback behavior and error handling when the `AuthID` changes unexpectedly. - Ensured that the `AuthID` is included in the response payload for better tracking and debugging.
Shared livepeerci/build:cache caused PR Docker tags (pr-3947, feat-add-model-id-signer-kafka) to point at an older livepeer binary while metadata listed the PR head SHA. Skip registry cache on pull_request builds and pass GIT_REVISION to bust the compile layer.
The remote signer hardcoded the create_signed_ticket `pipeline` to the lv2v constant whenever the gateway sent `type:"lv2v"` (which BYOC jobs always do for fee/pixel routing), and derived `model_id` only from the LiveVideoToVideo capability constraints. For BYOC capabilities (e.g. nano-banana) this emitted pipeline=live-video-to-video and an empty model_id, which the OpenMeter collector recorded as live-video-to-video / unknown. Add two additive, backward-compatible fields to RemotePaymentRequest: `capability` and `model_id`. When set by the gateway, they override the metered pipeline + model_id labels, decoupling usage attribution from `Type` (which still drives fee/pixel routing). When empty, behavior is byte-identical to before (lv2v constant + capabilities-derived model id; collector defaults empties to "unknown"). Label resolution is extracted into a pure resolveUsageLabels helper with a hermetic table test (TestResolveUsageLabels) that runs without the CGO ffmpeg toolchain.
The remote signer's GenerateLivePayment charged every BYOC generation a flat, model-independent fee: it read only oInfo.PriceInfo (the base price) and synthesized lv2v pixels (1280*720*30 * billableSecs), so nano-banana, recraft-v4, ltx-*, etc. all cost the same regardless of the per-capability prices the orchestrator already advertises in oInfo.CapabilitiesPrices. Resolve the fee per capability instead, keyed on req.Capability (added for metering by the stacked #3966): scan oInfo.CapabilitiesPrices for the Capability_BYOC entry whose Constraint matches the capability and charge that USD->wei/sec rate over compute-seconds (pixels = ceil(billableSecs)). This aligns the gateway's paid amount with the orchestrator's existing per-capability per-second accounting (byoc.JobPriceInfo * seconds). The resolved price is written back to oInfo.PriceInfo so it is the single source for state init, initialPrice, the max-price ceiling, the payment's ExpectedPrice (which the orch uses to set its fixed per-session price), and the price-doubling guard in validatePrice (which reads sess.OrchestratorInfo.PriceInfo) -- keeping all of them cap-vs-cap consistent and preventing a false "price more than doubled" rejection when base and cap diverge. Gated behind a new default-OFF flag (-byocPerCapPricing / LivepeerNode.ByocPerCapPricing). When OFF, or when req.Capability is empty, or when no usable cap price matches, behavior is byte-identical to the base-price path (zero regression). resolveByocPrice is a pure, hermetically testable function; ticket params (faceValue/winProb) are passed through unchanged so EV/ticket math stays valid. Tests: TestResolveByocPrice (resolution, fallback, non-BYOC/zero-rate ignored) and TestGenerateLivePayment_ByocPerCapPricing (flag-off zero-regression, per-cap seconds fee, 2:1 tariff ratio, unknown-cap fallback, doubling-guard not tripped), both CGO-free. Co-authored-by: Cursor <cursoragent@cursor.com>
Addresses Copilot review feedback on PR #3967: - resolveByocPrice now skips a matched-but-invalid (non-positive rate) entry and continues scanning instead of returning nil, so a later valid duplicate entry for the same constraint (e.g. from a misconfiguration) is still honored. Added a regression test case. (Copilot, remote_signer.go:431) - BYOC per-capability pricing is now additionally gated on Type==RemoteType_LiveVideoToVideo. Enabling it changes the billing basis (overrides req.InPixels and bypasses lv2v pixel synthesis), so it must only apply to the lv2v job type that BYOC jobs always use, never to a non-lv2v request that happens to carry a capability. (Copilot, remote_signer.go:488)
Addresses Copilot review feedback on PR #3966: - resolveUsageLabels now sanitizes the gateway-supplied capability/model_id override labels (TrimSpace + cap at 128 runes via sanitizeUsageLabel) before they are emitted to the create_signed_ticket usage event, bounding Kafka payload size and downstream label cardinality. req.Capability/req.ModelID come from the request body, so an unbounded value could otherwise inflate cardinality (pipeline was previously a small fixed set). - Corrected the resolveUsageLabels docstring: the capability/model_id overrides apply regardless of Type, not only for lv2v; clarified the empty-field fallback wording. - TestResolveUsageLabels now constructs a fresh require.New(t) inside each subtest so failures are attributed to the correct subtest, and adds cases for whitespace-only (ignored) and oversized (length-capped) override labels.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Jul 3, 2026
eliteprox
force-pushed
the
feat/add-model-id-signer-kafka
branch
from
July 8, 2026 19:41
82844a9 to
cfe860a
Compare
…pricing-and-usage-labels
…pricing-and-usage-labels
This was referenced Jul 9, 2026
7 tasks
Align staging orch with DMZ FlattenBYOCJob signing and accept explicit type:byoc on /generate-live-payment alongside the existing lv2v path. Co-authored-by: Cursor <cursoragent@cursor.com>
Changed the pipeline assignment in the remote signer to use the new capability-to-pipeline conversion function, ensuring that the correct pipeline is returned for BYOC capabilities. Updated the corresponding test to reflect this change in expected output.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Combines #3966 (usage-attribution labels) and #3967 (per-capability pricing) into a single PR. Both change-sets live entirely on the remote signer (
server/remote_signer.goGenerateLivePaymentand itscmd/livepeer/starterflag plumbing); no orchestrator code is touched. This PR supersedes and replaces #3966 and #3967, which are being closed in favor of this one.The two features are complementary and are both included in full:
Capability/ModelIDfields onRemotePaymentRequestoverride the meteredpipeline/model_idlabels on thecreate_signed_ticketusage event, decoupling usage attribution fromType(which still drives fee/pixel routing). Includes the addressed review fixes:sanitizeUsageLabel(TrimSpace + 128-rune cap viamaxUsageLabelLen) to bound Kafka payload size / label cardinality, corrected docstring, and hardenedTestResolveUsageLabels(per-subtestrequire, whitespace-only + oversized cases).resolveByocPriceresolves the BYOC fee from the orchestrator's advertisedOrchestratorInfo.CapabilitiesPrices(keyed onreq.Capability), gated behind the new default-OFF-byocPerCapPricingflag (LivepeerNode.ByocPerCapPricing). Includes the addressed review fixes: additionalType==lv2vgate on the pricing branch, and skip-invalid-duplicate-and-keep-scanning in the price resolver. The resolved price is written back tooInfo.PriceInfoso state init / initialPrice / max-price ceiling /ExpectedPrice/ thevalidatePricedoubling guard stay cap-vs-cap consistent.When the flag is OFF (default),
req.Capabilityis empty,Type != lv2v, or no usable cap price matches, behavior is byte-identical to the base-price path — zero regression.Provenance / review fixes carried over
Cherry-picked #3966 head
f7d2f83on top of #3967 head84c706a; the two editresolveUsageLabels/TestResolveUsageLabelscomplementarily and merged cleanly (no manual conflict). All previously-addressed Copilot review feedback on both #3966 and #3967 is preserved. Commits retain original authorship (incl. theCo-authored-by: Cursortrailer on the pricing feat commit).Test plan
resolveUsageLabels/sanitizeUsageLabel/maxUsageLabelLenpresent and used (labels union).resolveByocPrice/-byocPerCapPricing/Type==lv2vgate / invalid-duplicate skip present (pricing union).math,strings) present.TestResolveUsageLabels,TestResolveByocPrice,TestGenerateLivePayment_ByocPerCapPricing(local run blocked only by host ffmpeg/lpms CGO mismatch; validated via CI).Made with Cursor