Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
- Update to Rust 2024 Edition. ([#588](https://github.com/fastly/Viceroy/pull/588))
- Add stub implementations for bot detection hostcalls. ([#592](https://github.com/fastly/Viceroy/pull/592))
- Add no-op implementations for stale-if-error hostcalls ([#591](https://github.com/fastly/Viceroy/pull/591))
- Guest profiling support added for components. ([#593](https://github.com/fastly/Viceroy/pull/593))
- Add `manifest_version` validation to fastly.toml parsing. ([#590](https://github.com/fastly/Viceroy/pull/590))

## 0.16.4 (2026-01-26)
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,5 @@ anyhow = { workspace = true }
futures = { workspace = true }
url = { workspace = true }
tls-listener = { version = "^0.7.0", features = ["rustls", "hyper-h1", "tokio-net", "rt"] }
tempfile = "3"
serde_json = { workspace = true }
16 changes: 14 additions & 2 deletions cli/tests/integration/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ pub struct Test {
via_hyper: bool,
unknown_import_behavior: UnknownImportBehavior,
adapt_component: bool,
profiling_strategy: ProfilingStrategy,
guest_profile_config: Option<viceroy_lib::GuestProfileConfig>,
}

impl Test {
Expand All @@ -116,6 +118,8 @@ impl Test {
via_hyper: false,
unknown_import_behavior: Default::default(),
adapt_component: false,
profiling_strategy: ProfilingStrategy::None,
guest_profile_config: None,
}
}

Expand All @@ -140,6 +144,8 @@ impl Test {
via_hyper: false,
unknown_import_behavior: Default::default(),
adapt_component: false,
profiling_strategy: ProfilingStrategy::None,
guest_profile_config: None,
}
}

Expand Down Expand Up @@ -287,6 +293,12 @@ impl Test {
self
}

/// Enable guest profiling with the specified configuration.
pub fn with_guest_profiling(mut self, config: viceroy_lib::GuestProfileConfig) -> Self {
self.guest_profile_config = Some(config);
self
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there also be a with_profiling_strategy method for setting the profiling_strategy field?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I had plumbed that through but didn't write additional tests for those bits (as my changes are isolated to guest profiling). The naming and plumbing of some of these config bits is a bit confusing as the profiling strategy is unique to native profiling (not guest).

I have some changes now that consolidate things down to a single profiling_config which consolidates the guest/native/none paths in a single field. It's a fairly large change in terms of files touched, so I'm inclined to land this change as-is and then separately open up that change for review separately as it is beyond the scope of what we're changing here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


/// Pass the given requests through this test, returning the associated responses.
///
/// A `Test` can be used repeatedly against different requests, either individually (as with
Expand Down Expand Up @@ -331,9 +343,9 @@ impl Test {

let ctx = ExecuteCtx::build(
&self.module_path,
ProfilingStrategy::None,
self.profiling_strategy.clone(),
HashSet::new(),
None,
self.guest_profile_config.clone(),
self.unknown_import_behavior,
self.adapt_component,
)?
Expand Down
1 change: 1 addition & 0 deletions cli/tests/integration/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ mod invalid_status_code;
mod kv_store;
mod logging;
mod memory;
mod profiling;
mod request;
mod response;
mod reusable_sessions;
Expand Down
52 changes: 52 additions & 0 deletions cli/tests/integration/profiling.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
//! Tests for guest profiling functionality.

use {
crate::{
common::{Test, TestResult},
viceroy_test,
},
hyper::StatusCode,
std::time::Duration,
viceroy_lib::GuestProfileConfig,
};

viceroy_test!(guest_profiling_works, |is_component| {
// Create a temporary directory for the profile output
let temp_dir = tempfile::tempdir()?;
let profile_dir = temp_dir.path().join("profiles");
std::fs::create_dir(&profile_dir)?;

let resp = Test::using_fixture("noop.wasm")
.adapt_component(is_component)
.with_guest_profiling(GuestProfileConfig {
path: profile_dir.clone(),
sample_period: Duration::from_micros(50),
})
.against_empty()
.await?;

// Verify the request succeeded
assert_eq!(resp.status(), StatusCode::OK);

// Verify that a profile file was created in the directory
let profile_files: Vec<_> = std::fs::read_dir(&profile_dir)?
.filter_map(|e| e.ok())
.filter(|e| e.path().extension().and_then(|s| s.to_str()) == Some("json"))
.collect();

assert!(
!profile_files.is_empty(),
"At least one profile JSON file should be created in {:?}",
profile_dir
);

// Verify the first profile file has content and is valid JSON
let first_profile = &profile_files[0];
let metadata = std::fs::metadata(first_profile.path())?;
assert!(metadata.len() > 0, "Profile file should not be empty");

let profile_content = std::fs::read_to_string(first_profile.path())?;
let _: serde_json::Value = serde_json::from_str(&profile_content)?;

Ok(())
});
88 changes: 59 additions & 29 deletions src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,17 @@ const REGION_NONE: &str = "none";

enum Instance {
Module(Module, InstancePre<WasmCtx>),
Component(compute::bindings::AdapterServicePre<ComponentCtx>),
Component(
Component,
compute::bindings::AdapterServicePre<ComponentCtx>,
),
}

impl Instance {
fn unwrap_module(&self) -> (&Module, &InstancePre<WasmCtx>) {
match self {
Instance::Module(m, i) => (m, i),
Instance::Component(_) => panic!("unwrap_module called on a component"),
Instance::Component(_, _) => panic!("unwrap_module called on a component"),
}
}
}
Expand Down Expand Up @@ -245,7 +248,10 @@ impl ExecuteCtx {
}

let instance_pre = linker.instantiate_pre(&component)?;
Instance::Component(compute::bindings::AdapterServicePre::new(instance_pre)?)
Instance::Component(
component,
compute::bindings::AdapterServicePre::new(instance_pre)?,
)
} else {
let mut linker = Linker::new(&engine);
link_host_functions(&mut linker, &wasi_modules)?;
Expand Down Expand Up @@ -625,15 +631,21 @@ impl ExecuteCtx {
});

match self.instance_pre.as_ref() {
Instance::Component(instance_pre) => {
if self.guest_profile_config.is_some() {
warn!("Components do not currently support the guest profiler");
}
Instance::Component(component, instance_pre) => {
let profiler = self.guest_profile_config.as_deref().map(|pcfg| {
let program_name = "main";
GuestProfiler::new_component(
program_name,
pcfg.sample_period,
component.clone(),
std::iter::empty(),
)
});

let req = session.downstream_request();
let body = session.downstream_request_body();

let mut store = ComponentCtx::create_store(&self, session, None, |ctx| {
let mut store = ComponentCtx::create_store(&self, session, profiler, |ctx| {
ctx.arg("compute-app");
})
.map_err(ExecutionError::Context)?;
Expand Down Expand Up @@ -671,6 +683,9 @@ impl ExecuteCtx {
}
};

// If we collected a profile, write it to the file
write_profile_component(&mut store, guest_profile_path.as_ref());

// Ensure the downstream response channel is closed, whether or not a response was
// sent during execution.
let resp = outcome
Expand Down Expand Up @@ -793,7 +808,7 @@ impl ExecuteCtx {

let session = Session::new(downstream, active_cpu_time_us.clone(), self.clone());

if let Instance::Component(_) = self.instance_pre.as_ref() {
if let Instance::Component(_, _) = self.instance_pre.as_ref() {
panic!("components not currently supported with `run`");
}

Expand Down Expand Up @@ -884,7 +899,7 @@ impl ExecuteCtx {
}

pub fn is_component(&self) -> bool {
matches!(self.instance_pre.as_ref(), Instance::Component(_))
matches!(self.instance_pre.as_ref(), Instance::Component(_, _))
}
}

Expand Down Expand Up @@ -976,29 +991,44 @@ impl ExecuteCtxBuilder {
}
}

fn write_profile_to_file(profile: Box<GuestProfiler>, path: &PathBuf) {
match std::fs::File::create(path)
.map_err(anyhow::Error::new)
.and_then(|output| profile.finish(std::io::BufWriter::new(output)))
{
Err(e) => {
event!(
Level::ERROR,
"failed writing profile at {}: {e:#}",
path.display()
);
}
_ => {
event!(
Level::INFO,
"\nProfile written to: {}\nView this profile at https://profiler.firefox.com/.",
path.display()
);
}
}
}

fn write_profile(store: &mut wasmtime::Store<WasmCtx>, guest_profile_path: Option<&PathBuf>) {
if let (Some(profile), Some(path)) =
(store.data_mut().take_guest_profiler(), guest_profile_path)
{
match std::fs::File::create(path)
.map_err(anyhow::Error::new)
.and_then(|output| profile.finish(std::io::BufWriter::new(output)))
{
Err(e) => {
event!(
Level::ERROR,
"failed writing profile at {}: {e:#}",
path.display()
);
}
_ => {
event!(
Level::INFO,
"\nProfile written to: {}\nView this profile at https://profiler.firefox.com/.",
path.display()
);
}
}
write_profile_to_file(profile, path);
}
}

fn write_profile_component(
store: &mut wasmtime::Store<ComponentCtx>,
guest_profile_path: Option<&PathBuf>,
) {
if let (Some(profile), Some(path)) =
(store.data_mut().take_guest_profiler(), guest_profile_path)
{
write_profile_to_file(profile, path);
}
}

Expand Down
5 changes: 1 addition & 4 deletions wasm_abi/adapter/src/fastly/http_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,7 @@ mod http_cache {
STALE_WHILE_REVALIDATE_NS,
(*options).stale_while_revalidate_ns
),
stale_if_error_ns: when_enabled!(
STALE_IF_ERROR_NS,
(*options).stale_if_error_ns
),
stale_if_error_ns: when_enabled!(STALE_IF_ERROR_NS, (*options).stale_if_error_ns),
surrogate_keys,
length: when_enabled!(LENGTH, (*options).length),
sensitive_data: mask.contains(HttpCacheWriteOptionsMask::SENSITIVE_DATA),
Expand Down
Loading