feat: instrument dfx for telemetry #4149
Conversation
adamspofford-dfinity
force-pushed
the
spofford/telemetry-instrument
branch
from
81c0901 to
91450a5
Compare
adamspofford-dfinity
force-pushed
the
spofford/telemetry-instrument
branch
from
a4b5b5c to
31df56c
Compare
| @@ -117,6 +186,11 @@ impl Telemetry { | |||
|
|
|||
| pub fn append_current_command_timestamped(exit_code: i32) -> DfxResult<()> { | |||
| try_with_telemetry(|telemetry| { | |||
| let reject = telemetry.last_reject.as_ref(); | |||
| let call_site = telemetry.last_operation.as_ref().map(|o| match o { | |||
| Operation::Call { method, .. } => method, | |||
There was a problem hiding this comment.
For the functionality built in to dfx, it's okay to report the method name (notify_mint_cycles, create_batch). For user-specified method names, like dfx canister call <canister> <name>, we should report a placeholder rather than the actual name of the method. Something like literally <user-specified canister method>. The call site in this case is "calling the method that the user specified".
| # explicit call | ||
| dfx_start | ||
| assert_command_fail dfx canister call ryjl3-tyaaa-aaaaa-aaaba-cai name | ||
| assert_command jq -se 'last | .replica_error_call_site == "name" and .replica_error_code == "IC0301"' "$log" |
There was a problem hiding this comment.
This test is the one that shows we're reporting data that we should not.
| assert_command jq -se 'last | .replica_error_call_site == "name" and .replica_error_code == "IC0301"' "$log" | |
| assert_command jq -se 'last | .replica_error_call_site == "<user-specified canister method>" and .replica_error_code == "IC0301"' "$log" |
| LocalNetworkScopeDescriptor::Shared { .. } => NetworkType::LocalShared, | ||
| } | ||
| } else { | ||
| NetworkType::UnknownUrl |
There was a problem hiding this comment.
Can we set this up so the only way NetworkType::UnknownUrl comes up is if something like --network <url> was passed (and did not turn out to be is_ic)?
UnknownConfigured should mean a network config entry with providers (as opposed to bind), also that didn't turn out to be is_ic.
Technically, network.r#type doesn't divide networks into some kind of taxonomy. This field more describes how the canister ids are managed. You could have a local network that is persistent (though I don't know why you would), or a network with providers that is ephemeral. The playground has its own special handling.
Here's an attempt at identification:
- network.local_server_descriptor.is_some() - this is a local network. (comes from a config entry with
bind) - network.r#type is Playground - it's the playground (note this will have is_ic set too)
- network.is_ic - it's mainnet
- was passed as a url to
--network- UnknownUrl (would probably have to indicate this in create_url_based_network_descriptor()) - UnknownConfigured otherwise (came from a config entry with
providers, and did not turn out to be mainnet)
No description provided.