Skip to content

Commit

Permalink
feat: [IC-272] collect canister logs for update and replicated query …
Browse files Browse the repository at this point in the history
…calls
  • Loading branch information
maksymar committed Feb 21, 2024
1 parent f89bd76 commit 636d334
Show file tree
Hide file tree
Showing 12 changed files with 271 additions and 264 deletions.
3 changes: 3 additions & 0 deletions rs/canister_sandbox/src/sandbox_manager.rs
Expand Up @@ -138,6 +138,7 @@ impl Execution {
allocated_message_bytes,
instance_stats,
system_api_call_counters,
canister_log_records,
},
deltas,
instance_or_system_api,
Expand Down Expand Up @@ -204,6 +205,7 @@ impl Execution {
num_instructions_left,
instance_stats,
system_api_call_counters,
canister_log_records,
};
self.sandbox_manager.controller.execution_finished(
protocol::ctlsvc::ExecutionFinishedRequest {
Expand All @@ -230,6 +232,7 @@ impl Execution {
allocated_message_bytes,
instance_stats,
system_api_call_counters,
canister_log_records,
};

self.sandbox_manager.controller.execution_finished(
Expand Down
5 changes: 5 additions & 0 deletions rs/embedders/src/wasm_executor.rs
Expand Up @@ -455,6 +455,7 @@ pub fn wasm_execution_error(
allocated_message_bytes: NumBytes::from(0),
instance_stats: InstanceStats::default(),
system_api_call_counters: SystemApiCallCounters::default(),
canister_log_records: Default::default(),
},
None,
)
Expand Down Expand Up @@ -619,6 +620,7 @@ pub fn process(
allocated_message_bytes: NumBytes::from(0),
instance_stats: InstanceStats::default(),
system_api_call_counters: SystemApiCallCounters::default(),
canister_log_records: Default::default(),
},
None,
Err(system_api.unwrap()), // should be safe because we've passed Some(api) to new_instance
Expand All @@ -640,6 +642,7 @@ pub fn process(
//unwrap should not fail, because we have passed Some(system_api) to the instance above
let system_api = instance.store_data_mut().system_api_mut().unwrap();
let system_api_call_counters = system_api.call_counters();
let canister_log_records = system_api.canister_log_records().clone();
let slice_instruction_limit = system_api.slice_instruction_limit();
// Capping at the limit to preserve the existing behaviour. It should be
// possible to remove capping after ensuring that all callers can handle
Expand Down Expand Up @@ -676,6 +679,7 @@ pub fn process(
allocated_message_bytes: NumBytes::from(0),
instance_stats,
system_api_call_counters,
canister_log_records,
},
None,
Ok(instance),
Expand Down Expand Up @@ -780,6 +784,7 @@ pub fn process(
allocated_message_bytes,
instance_stats,
system_api_call_counters,
canister_log_records,
},
wasm_state_changes,
Ok(instance),
Expand Down
1 change: 1 addition & 0 deletions rs/execution_environment/src/execution/replicated_query.rs
Expand Up @@ -142,6 +142,7 @@ pub fn execute_replicated_query(
time,
);

canister.append_log_records(&output.canister_log_records);
let result = output.wasm_result;
let log = round.log;
let result = result.map_err(|err| err.into_user_error(&canister.canister_id()));
Expand Down
2 changes: 2 additions & 0 deletions rs/execution_environment/src/execution/update.rs
Expand Up @@ -367,6 +367,8 @@ impl UpdateHelper {
round_limits: &mut RoundLimits,
call_tree_metrics: &dyn CallTreeMetrics,
) -> ExecuteMessageResult {
self.canister
.append_log_records(&output.canister_log_records);
self.canister
.system_state
.apply_ingress_induction_cycles_debit(
Expand Down
10 changes: 2 additions & 8 deletions rs/execution_environment/src/execution_environment/tests.rs
Expand Up @@ -3285,10 +3285,7 @@ fn test_fetch_canister_logs_should_accept_ingress_message_disabled() {
let result = test.should_accept_ingress_message(
test.state().metadata.own_subnet_id.into(),
Method::FetchCanisterLogs,
FetchCanisterLogsRequest {
canister_id: canister_id.into(),
}
.encode(),
FetchCanisterLogsRequest::new(canister_id).encode(),
);
// Assert.
// Expect error because the API is disabled.
Expand Down Expand Up @@ -3318,10 +3315,7 @@ fn test_fetch_canister_logs_should_accept_ingress_message_enabled() {
let result = test.should_accept_ingress_message(
test.state().metadata.own_subnet_id.into(),
Method::FetchCanisterLogs,
FetchCanisterLogsRequest {
canister_id: canister_id.into(),
}
.encode(),
FetchCanisterLogsRequest::new(canister_id).encode(),
);
// Assert.
// Expect error since `should_accept_ingress_message` is only called in replicated mode which is not supported.
Expand Down
2 changes: 2 additions & 0 deletions rs/execution_environment/src/scheduler/test_utilities.rs
Expand Up @@ -1081,6 +1081,7 @@ impl TestWasmExecutorCore {
allocated_message_bytes: NumBytes::from(0),
instance_stats: InstanceStats::default(),
system_api_call_counters: SystemApiCallCounters::default(),
canister_log_records: Default::default(),
};
self.schedule
.push((self.round, canister_id, instructions_to_execute));
Expand Down Expand Up @@ -1126,6 +1127,7 @@ impl TestWasmExecutorCore {
num_instructions_left: instructions_left,
instance_stats,
system_api_call_counters: SystemApiCallCounters::default(),
canister_log_records: Default::default(),
};
self.schedule
.push((self.round, canister_id, instructions_to_execute));
Expand Down

0 comments on commit 636d334

Please sign in to comment.