Add Tags and Fields enums. #8
Conversation
|
For completeness I'll also implement |
There was a problem hiding this comment.
Look good.
To be sure though, is the plan for these to be used as follows?
trait Span {
// ... snip ...
fn log<F: Into<String>>(field: F, value: Value) {
let field: String = field.into();
// ... snip ...
}
}
fn main() {
let mut span = SomeSpan::new();
span.log(Fields::Event, "some event");
}
daschl
force-pushed
the
tags-and-fields
branch
from
316e768 to
8641e05
Compare
|
@stefano-pogliani fleshed out with errors and conversion traits.. its now pretty easy (see unit tests) how to convert and from, so it gives us lots of flexibility on how to use them |
opentracing-api/src/field.rs
Outdated
| fn cause(&self) -> Option<&Error> { | ||
| None | ||
| } | ||
|
|
There was a problem hiding this comment.
Nit-pick: newline at the end and start of this block is inconsistent with other blocks.
opentracing-api/src/field.rs
Outdated
| assert_eq!(Ok(Fields::Message), "message".parse()); | ||
| assert_eq!(Err(ParseFieldsError::UnknownField), Fields::from_str("some_other_field")); | ||
| } | ||
|
|
There was a problem hiding this comment.
Same here: block has new line at end and start that most blocks do not have.
opentracing-api/src/field.rs
Outdated
| /// The following log fields are recommended for instrumentors who are trying to capture more | ||
| /// information about a logged event. Tracers may expose additional features based on these | ||
| /// standardized data points. | ||
| #[derive(Debug, PartialEq)] |
There was a problem hiding this comment.
Just a couple of days ago I found the rust api guidelines which has a section about derives.
I think we want to follow those guidelines, what do you think?
There was a problem hiding this comment.
agreed, will add them!
|
@stefano-pogliani which traits do you think we should implement as well? I found derive, and partialeq sufficient - but happy to add more! |
|
That is a good question. Following the philosophy of "Who am I to dismiss the experience of the rust's lib team?" I'd say as many as possible? What do you think @daschl? |
This changeset adds the tags and fields enums as they are currently represented in the java implementation. Completes #7
|
@stefano-pogliani added all the tags which I think make sense (also checked with for example std::Result which also implements those, so we can't be too way off here) |
This changeset adds the tags and fields enums as they are
currently represented in the java implementation.
Completes #7