Skip to content

Commit

Permalink
api: replace StatusCode and message with Status
Browse files Browse the repository at this point in the history
The spec defines status as unset, ok, or error with a description. The
current api allows for confusion and misuse by providing invalid
combinations (e.g. Ok with a description). This change simplifies the
api and removes these illegal states by moving the status to be an enum
where only the `Status::Error` case accepts a message.
  • Loading branch information
jtescher committed Mar 13, 2022
1 parent 7c4545a commit dfd1b34
Show file tree
Hide file tree
Showing 23 changed files with 255 additions and 313 deletions.
4 changes: 2 additions & 2 deletions examples/zpages/src/main.rs
@@ -1,7 +1,7 @@
use hyper::http::{Request, Response};
use hyper::service::{make_service_fn, service_fn};
use hyper::{Body, Server};
use opentelemetry::trace::{Span, StatusCode};
use opentelemetry::trace::{Span, Status};
use opentelemetry::{global, runtime::Tokio, sdk::trace, trace::Tracer};
use opentelemetry_zpages::{tracez, TracezError, TracezQuerier, TracezResponse};
use rand::Rng;
Expand Down Expand Up @@ -66,7 +66,7 @@ async fn handler(
"/running" => {
let span_duration = Duration::from_millis(rand::thread_rng().gen_range(1..6000));
let mut spans = global::tracer("zpages-test").start("running-spans");
spans.set_status(StatusCode::Ok, "".to_string());
spans.set_status(Status::Ok);
tokio::time::sleep(span_duration).await;
println!("The span slept for {} ms", span_duration.as_millis());
Response::new(Body::empty())
Expand Down
15 changes: 6 additions & 9 deletions opentelemetry-api/src/global/trace.rs
@@ -1,5 +1,5 @@
use crate::global::handle_error;
use crate::trace::{noop::NoopTracerProvider, SpanContext, StatusCode, TraceResult};
use crate::trace::{noop::NoopTracerProvider, SpanContext, Status, TraceResult};
use crate::{trace, trace::TracerProvider, Context, KeyValue};
use std::borrow::Cow;
use std::fmt;
Expand Down Expand Up @@ -67,7 +67,7 @@ pub trait ObjectSafeSpan {
/// to `Unset` will always be ignore, set the status to `Error` only works when current
/// status is `Unset`, set the status to `Ok` will be consider final and any further call
/// to this function will be ignore.
fn set_status(&mut self, code: StatusCode, message: Cow<'static, str>);
fn set_status(&mut self, status: Status);

/// Updates the `Span`'s name. After this update, any sampling behavior based on the
/// name will depend on the implementation.
Expand Down Expand Up @@ -127,8 +127,8 @@ impl<T: trace::Span> ObjectSafeSpan for T {
self.set_attribute(attribute)
}

fn set_status(&mut self, code: StatusCode, message: Cow<'static, str>) {
self.set_status(code, message)
fn set_status(&mut self, status: Status) {
self.set_status(status)
}

fn update_name(&mut self, new_name: Cow<'static, str>) {
Expand Down Expand Up @@ -201,11 +201,8 @@ impl trace::Span for BoxedSpan {

/// Sets the status of the `Span`. If used, this will override the default `Span`
/// status, which is `Unset`.
fn set_status<T>(&mut self, code: trace::StatusCode, message: T)
where
T: Into<Cow<'static, str>>,
{
self.0.set_status(code, message.into())
fn set_status(&mut self, status: trace::Status) {
self.0.set_status(status)
}

/// Updates the `Span`'s name.
Expand Down
8 changes: 2 additions & 6 deletions opentelemetry-api/src/testing/trace.rs
@@ -1,5 +1,5 @@
use crate::{
trace::{Span, SpanContext, SpanId, StatusCode, TraceId},
trace::{Span, SpanContext, SpanId, Status, TraceId},
KeyValue,
};
use std::borrow::Cow;
Expand All @@ -24,11 +24,7 @@ impl Span for TestSpan {
false
}
fn set_attribute(&mut self, _attribute: KeyValue) {}
fn set_status<T>(&mut self, _code: StatusCode, _message: T)
where
T: Into<Cow<'static, str>>,
{
}
fn set_status(&mut self, _status: Status) {}
fn update_name<T>(&mut self, _new_name: T)
where
T: Into<Cow<'static, str>>,
Expand Down
13 changes: 4 additions & 9 deletions opentelemetry-api/src/trace/context.rs
@@ -1,7 +1,7 @@
//! Context extensions for tracing
use crate::{
global,
trace::{Span, SpanContext},
trace::{Span, SpanContext, Status},
Context, ContextGuard, KeyValue,
};
use futures_util::{sink::Sink, stream::Stream};
Expand Down Expand Up @@ -131,14 +131,9 @@ impl SpanRef<'_> {

/// Sets the status of this `Span`.
///
/// If used, this will override the default span status, which is [`StatusCode::Unset`].
///
/// [`StatusCode::Unset`]: super::StatusCode::Unset
pub fn set_status<T>(&self, code: super::StatusCode, message: T)
where
T: Into<Cow<'static, str>>,
{
self.with_inner_mut(move |inner| inner.set_status(code, message))
/// If used, this will override the default span status, which is [`Status::Unset`].
pub fn set_status(&self, status: Status) {
self.with_inner_mut(move |inner| inner.set_status(status))
}

/// Updates the span's name.
Expand Down
6 changes: 3 additions & 3 deletions opentelemetry-api/src/trace/mod.rs
Expand Up @@ -68,14 +68,14 @@
//! [`Context`]: crate::Context
//!
//! ```
//! use opentelemetry_api::{global, trace::{self, Span, StatusCode, Tracer, TracerProvider}};
//! use opentelemetry_api::{global, trace::{self, Span, Status, Tracer, TracerProvider}};
//!
//! fn may_error(rand: f32) {
//! if rand < 0.5 {
//! // Get the currently active span to record additional attributes,
//! // status, etc.
//! trace::get_active_span(|span| {
//! span.set_status(StatusCode::Error, "value too small");
//! span.set_status(Status::error("value too small"));
//! });
//! }
//! }
Expand Down Expand Up @@ -144,7 +144,7 @@ mod tracer_provider;

pub use self::{
context::{get_active_span, mark_span_as_active, FutureExt, SpanRef, TraceContextExt},
span::{Span, SpanKind, StatusCode},
span::{Span, SpanKind, Status},
span_context::{SpanContext, SpanId, TraceFlags, TraceId, TraceState},
tracer::{SamplingDecision, SamplingResult, SpanBuilder, Tracer},
tracer_provider::TracerProvider,
Expand Down
5 changes: 1 addition & 4 deletions opentelemetry-api/src/trace/noop.rs
Expand Up @@ -109,10 +109,7 @@ impl trace::Span for NoopSpan {
}

/// Ignores status
fn set_status<T>(&mut self, _code: trace::StatusCode, _message: T)
where
T: Into<Cow<'static, str>>,
{
fn set_status(&mut self, _status: trace::Status) {
// Ignored
}

Expand Down
83 changes: 62 additions & 21 deletions opentelemetry-api/src/trace/span.rs
Expand Up @@ -122,10 +122,8 @@ pub trait Span {

/// Sets the status of this `Span`.
///
/// If used, this will override the default span status, which is [`StatusCode::Unset`].
fn set_status<T>(&mut self, code: StatusCode, message: T)
where
T: Into<Cow<'static, str>>;
/// If used, this will override the default span status, which is [`Status::Unset`].
fn set_status(&mut self, status: Status);

/// Updates the span's name.
///
Expand Down Expand Up @@ -219,38 +217,81 @@ pub enum SpanKind {
Internal,
}

/// The code representation of the status of a [`Span`].
/// The status of a [`Span`].
///
/// These values form a total order: Ok > Error > Unset. This means that setting
/// `StatusCode::Ok` will override any prior or future attempts to set a
/// status with `StatusCode::Error` or `StatusCode::Unset`.
/// `Status::Ok` will override any prior or future attempts to set a status with
/// `Status::Error` or `Status::Unset`.
///
/// The status code should remain unset, except for the following circumstances:
/// The status should remain unset, except for the following circumstances:
///
/// Generally, instrumentation libraries should not set the status code to
/// `StatusCode::Ok`, unless explicitly configured to do so. Instrumentation
/// Generally, instrumentation libraries should not set the code to
/// `Status::Ok`, unless explicitly configured to do so. Instrumentation
/// libraries should leave the status code as unset unless there is an error.
///
/// Application developers and operators may set the status code to `StatusCode::Ok`.
/// Application developers and operators may set the status code to
/// `Status::Ok`.
///
/// When span status is set to `StatusCode::Ok` it should be considered final and
/// When span status is set to `Status::Ok` it should be considered final and
/// any further attempts to change it should be ignored.
///
/// Analysis tools should respond to a `StatusCode::Ok` status by suppressing any
/// errors they would otherwise generate. For example, to suppress noisy errors such
/// as 404s.
/// Analysis tools should respond to a `Status::Ok` status by suppressing any
/// errors they would otherwise generate. For example, to suppress noisy errors
/// such as 404s.
///
/// Only the value of the last call will be recorded, and implementations are free
/// to ignore previous calls.
#[derive(Clone, Debug, PartialEq, Copy)]
pub enum StatusCode {
/// Only the value of the last call will be recorded, and implementations are
/// free to ignore previous calls.
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd)]
pub enum Status {
/// The default status.
Unset,

/// The operation contains an error.
Error {
/// The description of the error
description: Cow<'static, str>,
},

/// The operation has been validated by an application developer or operator to
/// have completed successfully.
Ok,
}

/// The operation contains an error.
Error,
impl Status {
/// Create a new error status with a given description.
///
/// # Examples
///
/// ```
/// use opentelemetry_api::trace::Status;
///
/// // record error with `str` description
/// let error_status = Status::error("something went wrong");
///
/// // or with `String` description
/// let error_status = Status::error(format!("too many foos: {}", 42));
/// # drop(error_status);
/// ```
pub fn error(description: impl Into<Cow<'static, str>>) -> Self {
Status::Error {
description: description.into(),
}
}
}

impl Default for Status {
fn default() -> Self {
Status::Unset
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn status_order() {
assert!(Status::Ok > Status::error(""));
assert!(Status::error("") > Status::Unset);
}
}
35 changes: 15 additions & 20 deletions opentelemetry-api/src/trace/tracer.rs
@@ -1,7 +1,5 @@
use crate::{
trace::{
Event, Link, Span, SpanId, SpanKind, StatusCode, TraceContextExt, TraceId, TraceState,
},
trace::{Event, Link, Span, SpanId, SpanKind, Status, TraceContextExt, TraceId, TraceState},
Context, KeyValue,
};
use std::borrow::Cow;
Expand Down Expand Up @@ -244,26 +242,34 @@ pub trait Tracer {
pub struct SpanBuilder {
/// Trace id, useful for integrations with external tracing systems.
pub trace_id: Option<TraceId>,

/// Span id, useful for integrations with external tracing systems.
pub span_id: Option<SpanId>,

/// Span kind
pub span_kind: Option<SpanKind>,

/// Span name
pub name: Cow<'static, str>,

/// Span start time
pub start_time: Option<SystemTime>,

/// Span end time
pub end_time: Option<SystemTime>,

/// Span attributes
pub attributes: Option<Vec<KeyValue>>,

/// Span events
pub events: Option<Vec<Event>>,

/// Span Links
pub links: Option<Vec<Link>>,
/// Span status code
pub status_code: Option<StatusCode>,
/// Span status message
pub status_message: Option<Cow<'static, str>>,

/// Span status
pub status: Status,

/// Sampling result
pub sampling_result: Option<SamplingResult>,
}
Expand Down Expand Up @@ -344,19 +350,8 @@ impl SpanBuilder {
}

/// Assign status code
pub fn with_status_code(self, code: StatusCode) -> Self {
SpanBuilder {
status_code: Some(code),
..self
}
}

/// Assign status message
pub fn with_status_message<T: Into<Cow<'static, str>>>(self, message: T) -> Self {
SpanBuilder {
status_message: Some(message.into()),
..self
}
pub fn with_status(self, status: Status) -> Self {
SpanBuilder { status, ..self }
}

/// Assign sampling result
Expand Down
5 changes: 2 additions & 3 deletions opentelemetry-datadog/src/exporter/model/mod.rs
Expand Up @@ -77,7 +77,7 @@ pub(crate) mod tests {
use opentelemetry::sdk;
use opentelemetry::sdk::InstrumentationLibrary;
use opentelemetry::{
trace::{SpanContext, SpanId, SpanKind, StatusCode, TraceFlags, TraceId, TraceState},
trace::{SpanContext, SpanId, SpanKind, Status, TraceFlags, TraceId, TraceState},
Key,
};
use std::time::{Duration, SystemTime};
Expand Down Expand Up @@ -115,8 +115,7 @@ pub(crate) mod tests {
attributes,
events,
links,
status_code: StatusCode::Ok,
status_message: "".into(),
status: Status::Ok,
resource: None,
instrumentation_lib: InstrumentationLibrary::new("component", None, None),
}
Expand Down
6 changes: 3 additions & 3 deletions opentelemetry-datadog/src/exporter/model/v03.rs
@@ -1,6 +1,6 @@
use crate::exporter::model::Error;
use opentelemetry::sdk::export::trace;
use opentelemetry::trace::StatusCode;
use opentelemetry::trace::Status;
use opentelemetry::{Key, Value};
use std::time::SystemTime;

Expand Down Expand Up @@ -73,8 +73,8 @@ pub(crate) fn encode(
rmp::encode::write_str(&mut encoded, "error")?;
rmp::encode::write_i32(
&mut encoded,
match span.status_code {
StatusCode::Error => 1,
match span.status {
Status::Error { .. } => 1,
_ => 0,
},
)?;
Expand Down
6 changes: 3 additions & 3 deletions opentelemetry-datadog/src/exporter/model/v05.rs
@@ -1,7 +1,7 @@
use crate::exporter::intern::StringInterner;
use crate::exporter::Error;
use opentelemetry::sdk::export::trace;
use opentelemetry::trace::StatusCode;
use opentelemetry::trace::Status;
use opentelemetry::{Key, Value};
use std::time::SystemTime;

Expand Down Expand Up @@ -126,8 +126,8 @@ fn encode_traces(
rmp::encode::write_i64(&mut encoded, duration)?;
rmp::encode::write_i32(
&mut encoded,
match span.status_code {
StatusCode::Error => 1,
match span.status {
Status::Error { .. } => 1,
_ => 0,
},
)?;
Expand Down

0 comments on commit dfd1b34

Please sign in to comment.