Skip to content

Commit

Permalink
feat(proto): represent readiness as bool (#103)
Browse files Browse the repository at this point in the history
Based on some [earlier feedback][1], this PR changes the proto
definition so readiness of a poll op is represented via a bool. The
relevant changes have been made to:
tokio-rs/tokio#4072

[1]: #77 (comment)

Signed-off-by: Zahari Dichev <zaharidichev@gmail.com>
  • Loading branch information
zaharidichev committed Aug 31, 2021
1 parent 3c04f65 commit ba95a38
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 34 deletions.
8 changes: 3 additions & 5 deletions console-subscriber/src/aggregator/mod.rs
Expand Up @@ -724,7 +724,7 @@ impl Aggregator {
op_name,
async_op_id,
task_id,
readiness,
is_ready,
} => {
let async_op_id = self.ids.id_for(async_op_id);
let resource_id = self.ids.id_for(resource_id);
Expand All @@ -735,9 +735,7 @@ impl Aggregator {
async_op_stats.task_id.get_or_insert(task_id);
async_op_stats.resource_id.get_or_insert(resource_id);

if readiness == proto::Readiness::Pending
&& async_op_stats.poll_stats.first_poll.is_none()
{
if !is_ready && async_op_stats.poll_stats.first_poll.is_none() {
async_op_stats.poll_stats.first_poll = Some(at);
}

Expand All @@ -747,7 +745,7 @@ impl Aggregator {
name: op_name,
task_id: Some(task_id.into()),
async_op_id: Some(async_op_id.into()),
readiness: readiness as i32,
is_ready,
};

self.all_poll_ops.push(poll_op.clone());
Expand Down
6 changes: 3 additions & 3 deletions console-subscriber/src/lib.rs
Expand Up @@ -142,7 +142,7 @@ enum Event {
op_name: String,
async_op_id: span::Id,
task_id: span::Id,
readiness: proto::Readiness,
is_ready: bool,
},
StateUpdate {
metadata: &'static Metadata<'static>,
Expand Down Expand Up @@ -407,7 +407,7 @@ where
Some(resource_id) if self.is_id_resource(resource_id, &ctx) => {
let mut poll_op_visitor = PollOpVisitor::default();
event.record(&mut poll_op_visitor);
if let Some((op_name, readiness)) = poll_op_visitor.result() {
if let Some((op_name, is_ready)) = poll_op_visitor.result() {
let task_and_async_op_ids = self.current_spans.get().and_then(|stack| {
let stack = stack.borrow();
let task_id =
Expand All @@ -426,7 +426,7 @@ where
op_name,
async_op_id,
task_id,
readiness,
is_ready,
});
}
// else poll op event should be emitted in the context of an async op and task spans
Expand Down
32 changes: 13 additions & 19 deletions console-subscriber/src/visitors.rs
Expand Up @@ -87,7 +87,7 @@ pub(crate) struct WakerVisitor {
#[derive(Default)]
pub(crate) struct PollOpVisitor {
op_name: Option<String>,
readiness: Option<proto::Readiness>,
is_ready: Option<bool>,
}

/// Used to extract the fields needed to construct
Expand Down Expand Up @@ -256,33 +256,27 @@ impl Visit for WakerVisitor {
impl PollOpVisitor {
pub(crate) const POLL_OP_EVENT_TARGET: &'static str = "runtime::resource::poll_op";
const OP_NAME_FIELD_NAME: &'static str = "op_name";
const OP_READINESS_FIELD_NAME: &'static str = "readiness";
const OP_READINESS_READY: &'static str = "ready";
const OP_READINESS_PENDING: &'static str = "pending";
const OP_READINESS_FIELD_NAME: &'static str = "is_ready";

pub(crate) fn result(self) -> Option<(String, proto::Readiness)> {
pub(crate) fn result(self) -> Option<(String, bool)> {
let op_name = self.op_name?;
let readiness = self.readiness?;
Some((op_name, readiness))
let is_ready = self.is_ready?;
Some((op_name, is_ready))
}
}

impl Visit for PollOpVisitor {
fn record_debug(&mut self, _: &field::Field, _: &dyn std::fmt::Debug) {}

fn record_bool(&mut self, field: &tracing_core::Field, value: bool) {
if field.name() == Self::OP_READINESS_FIELD_NAME {
self.is_ready = Some(value)
}
}

fn record_str(&mut self, field: &tracing_core::Field, value: &str) {
match field.name() {
Self::OP_NAME_FIELD_NAME => {
self.op_name = Some(value.to_string());
}
Self::OP_READINESS_FIELD_NAME => {
self.readiness = Some(match value {
Self::OP_READINESS_READY => proto::Readiness::Ready,
Self::OP_READINESS_PENDING => proto::Readiness::Pending,
_ => return,
});
}
_ => {}
if field.name() == Self::OP_NAME_FIELD_NAME {
self.op_name = Some(value.to_string());
}
}
}
Expand Down
6 changes: 0 additions & 6 deletions proto/common.proto
Expand Up @@ -122,9 +122,3 @@ message PollStats {
// not reflecting any inprogress polls.
google.protobuf.Duration busy_time = 6;
}

// Indicates the readiness of a pollable entity (i.e task, resource).
enum Readiness {
READY = 0;
PENDING = 1;
}
2 changes: 1 addition & 1 deletion proto/resources.proto
Expand Up @@ -87,5 +87,5 @@ message PollOp {
// Identifies the async op ID that this poll op is part of.
common.Id async_op_id = 6;
// Whether this poll op has returned with ready or pending.
common.Readiness readiness = 7;
bool is_ready = 7;
}

0 comments on commit ba95a38

Please sign in to comment.