Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Pass startup_time to telemetry #4069

Merged
merged 2 commits into from
Nov 9, 2019
Merged

Pass startup_time to telemetry #4069

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f287539
Pass startup_time to telemetry
tomaka Nov 9, 2019
b3e5100
Merge remote-tracking branch 'upstream/master' into startup-time
tomaka Nov 9, 2019
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
11 changes: 9 additions & 2 deletions core/service/src/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ use sr_primitives::traits::{
Block as BlockT, Extrinsic, ProvideRuntimeApi, NumberFor, One, Zero, Header, SaturatedConversion
};
use substrate_executor::{NativeExecutor, NativeExecutionDispatch};
use std::{io::{Read, Write, Seek}, marker::PhantomData, sync::Arc, sync::atomic::AtomicBool};
use std::{
io::{Read, Write, Seek},
marker::PhantomData, sync::Arc, sync::atomic::AtomicBool, time::SystemTime
};
use sysinfo::{get_current_pid, ProcessExt, System, SystemExt};
use tel::{telemetry, SUBSTRATE_INFO};
use transaction_pool::txpool::{self, ChainApi, Pool as TransactionPool};
Expand Down Expand Up @@ -187,7 +190,7 @@ where TGen: RuntimeGenesis, TCSExt: Extension {
client_db::DatabaseSettingsSrc::Custom(db.clone()),
},
};

client_db::new_client(
db_config,
executor,
Expand Down Expand Up @@ -1095,6 +1098,9 @@ ServiceBuilder<
endpoints,
wasm_external_transport: config.telemetry_external_transport.take(),
});
let startup_time = SystemTime::UNIX_EPOCH.elapsed()
.map(|dur| dur.as_millis())
Copy link
Member

Choose a reason for hiding this comment

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

Isn't seconds enough?

Copy link
Member

Choose a reason for hiding this comment

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

This would pass machine start time, right? We want node (process) run time.

Copy link
Member

Choose a reason for hiding this comment

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

Telemetry should show node running time regardless of when we connected to telemetry. E.g. if the node was started an hour ago, and it only connected to telemetry 15 minutes ago, it should still show "1 hour". For this to work we need to pass our real startup time I believe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code is called only once, when we build the service.
startup_time is then cloned (well, copy'ed) when we connect to the telemetry.

Copy link
Member

Choose a reason for hiding this comment

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

I misread it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't seconds enough?

@maciejhirsz told me milliseconds were better.

.unwrap_or(0);
let future = telemetry.clone()
.map(|ev| Ok::<_, ()>(ev))
.compat()
Expand All @@ -1109,6 +1115,7 @@ ServiceBuilder<
"config" => "",
"chain" => chain_name.clone(),
"authority" => is_authority,
"startup_time" => startup_time,
"network_id" => network_id.clone()
);

Expand Down