Skip to content

Commit

Permalink
log metric on calling UDF as a js function (#32643)
Browse files Browse the repository at this point in the history
GitOrigin-RevId: 6d857a7d2467ab02a188c8754eb383375d716aee
  • Loading branch information
ldanilek authored and Convex, Inc. committed Dec 27, 2024
1 parent 44c0f33 commit 4822dd4
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 3 deletions.
20 changes: 20 additions & 0 deletions crates/isolate/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,26 @@ pub fn op_timer(op_name: &str) -> StatusTimer {
t
}

register_convex_counter!(
ISOLATE_DIRECT_FUNCTION_CALL_TOTAL,
"Number of calls to registered UDFs as js functions"
);
fn log_direct_function_call() {
log_counter(&ISOLATE_DIRECT_FUNCTION_CALL_TOTAL, 1);
}

pub fn log_log_line(line: &str) {
// We log a console.warn line containing this link when a function is called
// directly. These are potentially problematic because it looks like arg and
// return values are being validated, and a new isolate is running the UDF,
// but actually the plain JS function is being called. If the non-isolated,
// non-validated behavior is intended, the helper function should be explicit.
if line.contains("https://docs.convex.dev/production/best-practices/#use-helper-functions-to-write-shared-code") {
tracing::warn!("Direct function call detected: '{line}'");
log_direct_function_call();
}
}

register_convex_histogram!(
UDF_SYSCALL_SECONDS,
"Duration of UDF syscall",
Expand Down
8 changes: 7 additions & 1 deletion crates/isolate/src/ops/console.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,20 @@ use common::{
log_lines::LogLevel,
};

use super::OpProvider;
use super::{
metrics,
OpProvider,
};

#[convex_macro::v8_op]
pub fn op_console_message<'b, P: OpProvider<'b>>(
provider: &mut P,
level: String,
messages: Vec<String>,
) -> anyhow::Result<()> {
for message in messages.iter() {
metrics::log_log_line(message);
}
provider.trace(level.parse()?, messages)?;
Ok(())
}
Expand Down
6 changes: 5 additions & 1 deletion crates/isolate/src/tests/adversarial.rs
Original file line number Diff line number Diff line change
Expand Up @@ -920,6 +920,10 @@ async fn test_invoke_function_directly(rt: TestRuntime) -> anyhow::Result<()> {
.pop()
.unwrap()
.to_pretty_string_test_only();
assert_contains(&log_line, "[WARN] 'Do not call Convex functions directly.");
assert_contains(
&log_line,
"[WARN] 'Convex functions should not directly call other Convex functions. Consider \
calling a helper function instead.",
);
Ok(())
}
2 changes: 1 addition & 1 deletion npm-packages/convex/src/server/impl/registration_impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ function dontCallDirectly(
) {
return (ctx: any, args: any) => {
globalThis.console.warn(
"Do not call Convex functions directly. " +
"Convex functions should not directly call other Convex functions. Consider calling a helper function instead. " +
`e.g. \`export const foo = ${funcType}(...); await foo(ctx);\` is not supported. ` +
"See https://docs.convex.dev/production/best-practices/#use-helper-functions-to-write-shared-code",
);
Expand Down

0 comments on commit 4822dd4

Please sign in to comment.