Conversation
Signed-off-by: Alexander Cristurean <acristur@redhat.com>
bc97846 to
89ef0cd
Compare
Signed-off-by: Alexander Cristurean <acristur@redhat.com>
Lock the fail-closed contract for the file-backed credential_inject filter across startup seeding, rotation under traffic, and deletion, driving the real provider-route pipeline in-process. Signed-off-by: Alexander Cristurean <acristur@redhat.com>
Signed-off-by: Alexander Cristurean <acristur@redhat.com>
praxis-bot
left a comment
There was a problem hiding this comment.
Review: feat(metering) — provider and reasoning tokens
The metering changes are clean, well-structured, and thoroughly tested. Provider resolution with static config > cluster name > empty fallback is correct and covers the ext-proc data path where cluster_name() is always None. Reasoning token plumbing from token_count metadata through TokenCounts into the CloudEvent is straightforward. Tests cover all three provider resolution branches, reasoning token round-trip, and the zero-default case.
One scope concern below.
Findings: 1
| @@ -0,0 +1,438 @@ | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
There was a problem hiding this comment.
[Medium] This 438-line file adds credential-store parity acceptance tests that are unrelated to the metering provider/reasoning-token feature. The PR description does not mention it, and the commit (ca94a64a) is a separate concern. Consider splitting this into its own PR so each change set can be reviewed and reverted independently.
Problem
Metering CloudEvents (
inference.tokens.used) were not fully configurable from the deployment. The provider/cluster attribute was derived fromctx.cluster_name(), but the ext-proc adapter hard-codescluster: Noneand there was no config field to set it — so the controller could not render a correct value. Reasoning tokens were also not surfaced in the usage event.See opendatahub-io/praxis-extproc#80.
Changes
providerconfig field to theexternal_meteringfilter so the emitted CloudEvents carry a statically configurable provider, independent ofctx.cluster_name().reasoning_tokensin the usage CloudEvent when present.examples/configs/external-metering.yaml, filter docs, unit tests, and the example integration suite to cover the new field and reasoning-token emission.Tests
filters/src/metering/tests.rs.Refs opendatahub-io/praxis-extproc#80