Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Fix metrics
  • Loading branch information
montekki committed Sep 21, 2020
commit 07ce6d604821527021650a6b5a29b88aa46e2335
4 changes: 2 additions & 2 deletions node/network/collator-protocol/src/collator_side.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,14 @@ impl metrics::Metrics for Metrics {
let metrics = MetricsInner {
advertisments_made: prometheus::register(
prometheus::Counter::new(
"advertisments_made_total",
"parachain_advertisments_made_total",
"A number of advertisments sent to validators.",
)?,
registry,
)?,
collations_sent: prometheus::register(
prometheus::Counter::new(
"collations_sent_total",
"parachain_collations_sent_total",
"A number of collations sent to validators.",
)?,
registry,
Expand Down
26 changes: 6 additions & 20 deletions node/network/collator-protocol/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,19 +64,6 @@ enum ProtocolSide {
Collator(CollatorId, collator_side::Metrics),
}

/// Metrics.
#[derive(Clone, Default)]
pub struct Metrics;

// TODO: So how do we tie this to metrics in modules?.
impl metrics::Metrics for Metrics {
fn try_register(_registry: &prometheus::Registry)
-> std::result::Result<Self, prometheus::PrometheusError>
{
Ok(Self)
}
}

/// The collator protocol subsystem.
pub struct CollatorProtocolSubsystem {
protocol_side: ProtocolSide,
Expand All @@ -87,17 +74,16 @@ impl CollatorProtocolSubsystem {
/// If `id` is `Some` this is a collator side of the protocol.
/// If `id` is `None` this is a validator side of the protocol.
/// Caller must provide a registry for prometheus metrics.
pub fn new(id: Option<CollatorId>, registry: &prometheus::Registry)
-> std::result::Result<Self, prometheus::PrometheusError> {
pub fn new(id: Option<CollatorId>, registry: Option<&prometheus::Registry>) -> Self {
use metrics::Metrics;
let protocol_side = match id {
Some(id) => ProtocolSide::Collator(id, collator_side::Metrics::try_register(registry)?),
None => ProtocolSide::Validator(validator_side::Metrics::try_register(registry)?),
Some(id) => ProtocolSide::Collator(id, collator_side::Metrics::register(registry)),
None => ProtocolSide::Validator(validator_side::Metrics::register(registry)),
};

Ok(Self {
Self {
protocol_side,
})
}
}

async fn run<Context>(self, ctx: Context) -> Result<()>
Expand All @@ -123,7 +109,7 @@ impl<Context> Subsystem<Context> for CollatorProtocolSubsystem
where
Context: SubsystemContext<Message = CollatorProtocolMessage> + Sync + Send,
{
type Metrics = Metrics;
type Metrics = ();
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really understand why we expose the Metrics type in the trait if it's ok to just fake it and use a different type internally. I assume this is fine as @ordian has already approved this PR, but it may be worth considering whether we could remove type Metrics from trait Subsystem.

If on the other hand there's a reason to publish this type, then we really should be distributing it via an enum.


fn start(self, ctx: Context) -> SpawnedSubsystem {
SpawnedSubsystem {
Expand Down
2 changes: 1 addition & 1 deletion node/network/collator-protocol/src/validator_side.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ impl metrics::Metrics for Metrics {
collation_requests: prometheus::register(
prometheus::CounterVec::new(
prometheus::Opts::new(
"collation_requests_total",
"parachain_collation_requests_total",
"Number of collations requested from Collators.",
),
&["succeeded", "failed"],
Expand Down