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.

Sign up for GitHub

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

Jump to bottom

api: rename Span::record_exception to Span::record_error #756

Merged
merged 1 commit into from Mar 13, 2022
Merged

api: rename Span::record_exception to Span::record_error #756

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 8 additions & 11 deletions opentelemetry-api/src/trace/context.rs
View file
Open in desktop
Expand Up @@ -60,17 +60,14 @@ impl SpanRef<'_> {
self.with_inner_mut(|inner| inner.add_event(name, attributes))
}

/// Record an exception event
pub fn record_exception(&self, err: &dyn Error) {
self.with_inner_mut(|inner| inner.record_exception(err))
}

/// Record an exception event with stacktrace
pub fn record_exception_with_stacktrace<T>(&self, err: &dyn Error, stacktrace: T)
where
T: Into<Cow<'static, str>>,
{
self.with_inner_mut(|inner| inner.record_exception_with_stacktrace(err, stacktrace))
/// Record an error as an event for this span.
///
/// An additional call to [Span::set_status] is required if the status of the
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of calling this out in the docs, why don't we just fix it?

Copy link
Member Author

Choose a reason for hiding this comment

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

It appears they are intended to be able to be called independently, I'd guess to support recording errors that are caught or otherwise not resulting in an error state for the trace. See the Java impl and the go impl.

/// span should be set to error, as this method does not change the span status.
///
/// If this span is not being recorded then this method does nothing.
pub fn record_error(&self, err: &dyn Error) {
self.with_inner_mut(|inner| inner.record_error(err))
}

/// Record an event with a timestamp in the context this span.
Expand Down
28 changes: 11 additions & 17 deletions opentelemetry-api/src/trace/span.rs
View file
Open in desktop
Expand Up @@ -64,23 +64,17 @@ pub trait Span {
self.add_event_with_timestamp(name, crate::time::now(), attributes)
}

/// Record an exception event
fn record_exception(&mut self, err: &dyn Error) {
let attributes = vec![KeyValue::new("exception.message", err.to_string())];
self.add_event("exception", attributes);
}

/// Record an exception event with stacktrace
fn record_exception_with_stacktrace<T>(&mut self, err: &dyn Error, stacktrace: T)
where
T: Into<Cow<'static, str>>,
{
let attributes = vec![
KeyValue::new("exception.message", err.to_string()),
KeyValue::new("exception.stacktrace", stacktrace.into()),
];

self.add_event("exception", attributes);
/// Record an error as an event for this span.
///
/// An additional call to [Span::set_status] is required if the status of the
/// span should be set to error, as this method does not change the span status.
///
/// If this span is not being recorded then this method does nothing.
fn record_error(&mut self, err: &dyn Error) {
if self.is_recording() {
let attributes = vec![KeyValue::new("exception.message", err.to_string())];
self.add_event("exception", attributes);
}
}

/// Record an event with a timestamp in the context this span.
Expand Down
29 changes: 3 additions & 26 deletions opentelemetry-sdk/src/trace/span.rs
View file
Open in desktop
Expand Up @@ -359,10 +359,10 @@ mod tests {
}

#[test]
fn record_exception() {
fn record_error() {
let mut span = create_span();
let err = std::io::Error::from(std::io::ErrorKind::Other);
span.record_exception(&err);
span.record_error(&err);
span.with_data(|data| {
if let Some(event) = data.events.iter().next() {
assert_eq!(event.name, "exception");
Expand All @@ -376,28 +376,6 @@ mod tests {
});
}

#[test]
fn record_exception_with_stacktrace() {
let mut span = create_span();
let err = std::io::Error::from(std::io::ErrorKind::Other);
let stacktrace = "stacktrace...".to_string();
span.record_exception_with_stacktrace(&err, stacktrace.clone());
span.with_data(|data| {
if let Some(event) = data.events.iter().next() {
assert_eq!(event.name, "exception");
assert_eq!(
event.attributes,
vec![
KeyValue::new("exception.message", err.to_string()),
KeyValue::new("exception.stacktrace", stacktrace),
]
);
} else {
panic!("no event");
}
});
}

#[test]
fn set_attribute() {
let mut span = create_span();
Expand Down Expand Up @@ -517,8 +495,7 @@ mod tests {
vec![KeyValue::new("k", "v")],
);
let err = std::io::Error::from(std::io::ErrorKind::Other);
span.record_exception(&err);
span.record_exception_with_stacktrace(&err, "stacktrace...".to_string());
span.record_error(&err);
span.set_attribute(KeyValue::new("k", "v"));
span.set_status(StatusCode::Error, "ERROR".to_string());
span.update_name("new_name".to_string());
Expand Down