Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proto: represent readiness as bool #103

Merged
merged 1 commit into from Aug 31, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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;
}