chore: add approval metric (#8970)
This commit is contained in:
parent
225614d7fb
commit
e2e3f4490e
5 changed files with 38 additions and 8 deletions
|
|
@ -111,12 +111,17 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
|
|||
return rx_approve.await.unwrap_or_default();
|
||||
}
|
||||
|
||||
with_cached_approval(&session.services, approval_keys, || async move {
|
||||
let rx_approve = session
|
||||
.request_patch_approval(turn, call_id, changes, None, None)
|
||||
.await;
|
||||
rx_approve.await.unwrap_or_default()
|
||||
})
|
||||
with_cached_approval(
|
||||
&session.services,
|
||||
"apply_patch",
|
||||
approval_keys,
|
||||
|| async move {
|
||||
let rx_approve = session
|
||||
.request_patch_approval(turn, call_id, changes, None, None)
|
||||
.await;
|
||||
rx_approve.await.unwrap_or_default()
|
||||
},
|
||||
)
|
||||
.await
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -98,7 +98,7 @@ impl Approvable<ShellRequest> for ShellRuntime {
|
|||
let turn = ctx.turn;
|
||||
let call_id = ctx.call_id.to_string();
|
||||
Box::pin(async move {
|
||||
with_cached_approval(&session.services, keys, move || async move {
|
||||
with_cached_approval(&session.services, "shell", keys, move || async move {
|
||||
session
|
||||
.request_command_approval(
|
||||
turn,
|
||||
|
|
|
|||
|
|
@ -116,7 +116,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
|||
.clone()
|
||||
.or_else(|| req.justification.clone());
|
||||
Box::pin(async move {
|
||||
with_cached_approval(&session.services, keys, || async move {
|
||||
with_cached_approval(&session.services, "unified_exec", keys, || async move {
|
||||
session
|
||||
.request_command_approval(
|
||||
turn,
|
||||
|
|
|
|||
|
|
@ -57,6 +57,8 @@ impl ApprovalStore {
|
|||
/// so future requests touching any subset can also skip prompting.
|
||||
pub(crate) async fn with_cached_approval<K, F, Fut>(
|
||||
services: &SessionServices,
|
||||
// Name of the tool, used for metrics collection.
|
||||
tool_name: &str,
|
||||
keys: Vec<K>,
|
||||
fetch: F,
|
||||
) -> ReviewDecision
|
||||
|
|
@ -82,6 +84,15 @@ where
|
|||
|
||||
let decision = fetch().await;
|
||||
|
||||
services.otel_manager.counter(
|
||||
"codex.approval.requested",
|
||||
1,
|
||||
&[
|
||||
("tool", tool_name),
|
||||
("approved", decision.to_opaque_string()),
|
||||
],
|
||||
);
|
||||
|
||||
if matches!(decision, ReviewDecision::ApprovedForSession) {
|
||||
let mut store = services.tool_approvals.lock().await;
|
||||
for key in keys {
|
||||
|
|
|
|||
|
|
@ -1878,6 +1878,20 @@ pub enum ReviewDecision {
|
|||
Abort,
|
||||
}
|
||||
|
||||
impl ReviewDecision {
|
||||
/// Returns an opaque version of the decision without PII. We can't use an ignored flag
|
||||
/// on `serde` because the serialization is required by some surfaces.
|
||||
pub fn to_opaque_string(&self) -> &'static str {
|
||||
match self {
|
||||
ReviewDecision::Approved => "approved",
|
||||
ReviewDecision::ApprovedExecpolicyAmendment { .. } => "approved_with_amendment",
|
||||
ReviewDecision::ApprovedForSession => "approved_for_session",
|
||||
ReviewDecision::Denied => "denied",
|
||||
ReviewDecision::Abort => "abort",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, JsonSchema, TS)]
|
||||
#[serde(tag = "type", rename_all = "snake_case")]
|
||||
#[ts(tag = "type")]
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue