-
Notifications
You must be signed in to change notification settings - Fork 378
Complete telemetry for parachain & relaychain #301
Conversation
rococo-parachains/src/service.rs
Outdated
@@ -126,6 +126,7 @@ where | |||
|
|||
let params = new_partial(¶chain_config)?; | |||
let telemetry_span = params.other; | |||
let _telemetry_span_entered = telemetry_span.as_ref().map(|x| x.enter()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to do this here manually?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to investigate but I think there is something in a function somewhere below that reports something to the telemetry.
I know it's vague and it's probably not needed for the telemetry to work. I just wanted to make sure we don't miss a telemetry message at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please investigate this. The problem here is that this isn't any code that is reusable, so people would need to copy this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right!! I missed that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed by paritytech/substrate#7951
36c7dfb
to
4126e0e
Compare
@bkchr this is also ready for review |
@cecton fails to compile :P |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not import Encode
from sp_core
. Otherwise it looks good.
Closes #149
Closes #154