Skip to content

Commit

Permalink
Simplify global impl and add documentation (#79)
Browse files Browse the repository at this point in the history
This patch simplifies the global module by implementing
`GenericProvider` and `GenericTracer` on impls of `Provider` and
`Tracer` directly. These changes remove multiple levels of boxing that
were necessary with the previous implementation.
  • Loading branch information
jtescher authored Mar 24, 2020
1 parent 07ad1e6 commit 2c687c5
Show file tree
Hide file tree
Showing 17 changed files with 261 additions and 204 deletions.
6 changes: 3 additions & 3 deletions opentelemetry-jaeger/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ impl<T: net::ToSocketAddrs> Builder<T> {

/// Assign the collector endpoint.
#[cfg(feature = "collector_client")]
pub fn with_collector_endpoint<T: Into<String>>(self, collector_endpoint: T) -> Self {
pub fn with_collector_endpoint<S: Into<String>>(self, collector_endpoint: S) -> Self {
Builder {
collector_endpoint: Some(collector_endpoint.into()),
..self
Expand All @@ -226,7 +226,7 @@ impl<T: net::ToSocketAddrs> Builder<T> {

/// Assign the collector username
#[cfg(feature = "collector_client")]
pub fn with_collector_username<T: Into<String>>(self, collector_username: T) -> Self {
pub fn with_collector_username<S: Into<String>>(self, collector_username: S) -> Self {
Builder {
collector_username: Some(collector_username.into()),
..self
Expand All @@ -235,7 +235,7 @@ impl<T: net::ToSocketAddrs> Builder<T> {

/// Assign the collector password
#[cfg(feature = "collector_client")]
pub fn with_collector_password<T: Into<String>>(self, collector_password: T) -> Self {
pub fn with_collector_password<S: Into<String>>(self, collector_password: S) -> Self {
Builder {
collector_password: Some(collector_password.into()),
..self
Expand Down
4 changes: 2 additions & 2 deletions opentelemetry-jaeger/src/thrift/agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ impl <C: TThriftClient + TAgentSyncClientMarker> TAgentSyncClient for C {
{
self.increment_sequence_number();
let message_ident = TMessageIdentifier::new("emitZipkinBatch", TMessageType::OneWay, self.sequence_number());
let call_args = AgentEmitZipkinBatchArgs { spans: spans };
let call_args = AgentEmitZipkinBatchArgs { spans };
self.o_prot_mut().write_message_begin(&message_ident)?;
call_args.write_to_out_protocol(self.o_prot_mut())?;
self.o_prot_mut().write_message_end()?;
Expand All @@ -82,7 +82,7 @@ impl <C: TThriftClient + TAgentSyncClientMarker> TAgentSyncClient for C {
{
self.increment_sequence_number();
let message_ident = TMessageIdentifier::new("emitBatch", TMessageType::OneWay, self.sequence_number());
let call_args = AgentEmitBatchArgs { batch: batch };
let call_args = AgentEmitBatchArgs { batch };
self.o_prot_mut().write_message_begin(&message_ident)?;
call_args.write_to_out_protocol(self.o_prot_mut())?;
self.o_prot_mut().write_message_end()?;
Expand Down
42 changes: 21 additions & 21 deletions opentelemetry-jaeger/src/thrift/jaeger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ pub struct Tag {
impl Tag {
pub fn new<F3, F4, F5, F6, F7>(key: String, v_type: TagType, v_str: F3, v_double: F4, v_bool: F5, v_long: F6, v_binary: F7) -> Tag where F3: Into<Option<String>>, F4: Into<Option<OrderedFloat<f64>>>, F5: Into<Option<bool>>, F6: Into<Option<i64>>, F7: Into<Option<Vec<u8>>> {
Tag {
key: key,
v_type: v_type,
key,
v_type,
v_str: v_str.into(),
v_double: v_double.into(),
v_bool: v_bool.into(),
Expand Down Expand Up @@ -259,8 +259,8 @@ pub struct Log {
impl Log {
pub fn new(timestamp: i64, fields: Vec<Tag>) -> Log {
Log {
timestamp: timestamp,
fields: fields,
timestamp,
fields,
}
}
pub fn read_from_in_protocol(i_prot: &mut dyn TInputProtocol) -> thrift::Result<Log> {
Expand Down Expand Up @@ -336,10 +336,10 @@ pub struct SpanRef {
impl SpanRef {
pub fn new(ref_type: SpanRefType, trace_id_low: i64, trace_id_high: i64, span_id: i64) -> SpanRef {
SpanRef {
ref_type: ref_type,
trace_id_low: trace_id_low,
trace_id_high: trace_id_high,
span_id: span_id,
ref_type,
trace_id_low,
trace_id_high,
span_id,
}
}
pub fn read_from_in_protocol(i_prot: &mut dyn TInputProtocol) -> thrift::Result<SpanRef> {
Expand Down Expand Up @@ -432,15 +432,15 @@ pub struct Span {
impl Span {
pub fn new<F6, F10, F11>(trace_id_low: i64, trace_id_high: i64, span_id: i64, parent_span_id: i64, operation_name: String, references: F6, flags: i32, start_time: i64, duration: i64, tags: F10, logs: F11) -> Span where F6: Into<Option<Vec<SpanRef>>>, F10: Into<Option<Vec<Tag>>>, F11: Into<Option<Vec<Log>>> {
Span {
trace_id_low: trace_id_low,
trace_id_high: trace_id_high,
span_id: span_id,
parent_span_id: parent_span_id,
operation_name: operation_name,
trace_id_low,
trace_id_high,
span_id,
parent_span_id,
operation_name,
references: references.into(),
flags: flags,
start_time: start_time,
duration: duration,
flags,
start_time,
duration,
tags: tags.into(),
logs: logs.into(),
}
Expand Down Expand Up @@ -638,7 +638,7 @@ pub struct Process {
impl Process {
pub fn new<F2>(service_name: String, tags: F2) -> Process where F2: Into<Option<Vec<Tag>>> {
Process {
service_name: service_name,
service_name,
tags: tags.into(),
}
}
Expand Down Expand Up @@ -717,8 +717,8 @@ pub struct Batch {
impl Batch {
pub fn new(process: Process, spans: Vec<Span>) -> Batch {
Batch {
process: process,
spans: spans,
process,
spans,
}
}
pub fn read_from_in_protocol(i_prot: &mut dyn TInputProtocol) -> thrift::Result<Batch> {
Expand Down Expand Up @@ -791,7 +791,7 @@ pub struct BatchSubmitResponse {
impl BatchSubmitResponse {
pub fn new(ok: bool) -> BatchSubmitResponse {
BatchSubmitResponse {
ok: ok,
ok,
}
}
pub fn read_from_in_protocol(i_prot: &mut dyn TInputProtocol) -> thrift::Result<BatchSubmitResponse> {
Expand Down Expand Up @@ -869,7 +869,7 @@ impl <C: TThriftClient + TCollectorSyncClientMarker> TCollectorSyncClient for C
{
self.increment_sequence_number();
let message_ident = TMessageIdentifier::new("submitBatches", TMessageType::Call, self.sequence_number());
let call_args = CollectorSubmitBatchesArgs { batches: batches };
let call_args = CollectorSubmitBatchesArgs { batches };
self.o_prot_mut().write_message_begin(&message_ident)?;
call_args.write_to_out_protocol(self.o_prot_mut())?;
self.o_prot_mut().write_message_end()?;
Expand Down
5 changes: 3 additions & 2 deletions opentelemetry-jaeger/src/thrift/zipkincore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,8 @@ pub struct Response {
impl Response {
pub fn new(ok: bool) -> Response {
Response {
ok: ok,
ok,

}
}
pub fn read_from_in_protocol(i_prot: &mut dyn TInputProtocol) -> thrift::Result<Response> {
Expand Down Expand Up @@ -845,7 +846,7 @@ impl <C: TThriftClient + TZipkinCollectorSyncClientMarker> TZipkinCollectorSyncC
{
self.increment_sequence_number();
let message_ident = TMessageIdentifier::new("submitZipkinBatch", TMessageType::Call, self.sequence_number());
let call_args = ZipkinCollectorSubmitZipkinBatchArgs { spans: spans };
let call_args = ZipkinCollectorSubmitZipkinBatchArgs { spans };
self.o_prot_mut().write_message_begin(&message_ident)?;
call_args.write_to_out_protocol(self.o_prot_mut())?;
self.o_prot_mut().write_message_end()?;
Expand Down
11 changes: 5 additions & 6 deletions opentelemetry-jaeger/src/transport/http.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//! Thrift HTTP transport
use reqwest::header;
use std::error::Error;
use std::fmt;
use std::sync::{Arc, Mutex};
use thrift::{TransportError, TransportErrorKind};
Expand Down Expand Up @@ -48,7 +47,7 @@ impl THttpChannel {
.map_err(|err| {
thrift::Error::Transport(TransportError::new(
TransportErrorKind::Unknown,
err.description(),
err.to_string(),
))
})?;

Expand Down Expand Up @@ -87,7 +86,7 @@ impl std::io::Read for THttpChannel {
let mut data = self
.read_buffer
.lock()
.map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err.description()))?;
.map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err.to_string()))?;

let amt = data.as_slice().read(buf)?;
if amt > 0 {
Expand All @@ -109,7 +108,7 @@ impl std::io::Write for THttpChannel {
let mut write_buffer = self
.write_buffer
.lock()
.map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err.description()))?;
.map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err.to_string()))?;
write_buffer.extend_from_slice(buf);

Ok(buf.len())
Expand All @@ -120,7 +119,7 @@ impl std::io::Write for THttpChannel {
let mut write_buffer = self
.write_buffer
.lock()
.map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err.description()))?;
.map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err.to_string()))?;
let mut req = self.client.post(&self.endpoint).body(write_buffer.clone());
if let (Some(username), Some(password)) = (self.username.as_ref(), self.password.as_ref()) {
req = req.basic_auth(username, Some(password));
Expand All @@ -142,7 +141,7 @@ impl std::io::Write for THttpChannel {
let mut read_buffer = self
.read_buffer
.lock()
.map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err.description()))?;
.map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err.to_string()))?;

resp.copy_to(&mut *read_buffer)
.map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err))?;
Expand Down
3 changes: 3 additions & 0 deletions opentelemetry-zipkin/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
//! }
//! ```
//!
#![deny(missing_docs, unreachable_pub, missing_debug_implementations)]
#![cfg_attr(test, deny(warnings))]

#[macro_use]
extern crate typed_builder;
Expand Down Expand Up @@ -83,6 +85,7 @@ impl Default for ExporterConfigBuilder {
}

impl ExporterConfig {
/// Create an export config builder
pub fn builder() -> ExporterConfigBuilder {
ExporterConfigBuilder::default()
}
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-zipkin/src/model/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::net::{Ipv4Addr, Ipv6Addr};

#[derive(TypedBuilder, Clone, Debug, Serialize)]
#[serde(rename_all = "camelCase")]
pub struct Endpoint {
pub(crate) struct Endpoint {
#[builder(setter(strip_option), default)]
#[serde(skip_serializing_if = "Option::is_none")]
service_name: Option<String>,
Expand Down
6 changes: 3 additions & 3 deletions opentelemetry-zipkin/src/model/span.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ use serde::Serialize;
use std::collections::HashMap;

#[derive(Serialize)]
pub struct ListOfSpans(pub(crate) Vec<Span>);
pub(crate) struct ListOfSpans(pub(crate) Vec<Span>);

#[derive(Clone, Debug, Serialize)]
#[serde(rename_all = "SCREAMING_SNAKE_CASE")]
pub enum Kind {
pub(crate) enum Kind {
Client,
Server,
Producer,
Expand All @@ -16,7 +16,7 @@ pub enum Kind {

#[derive(TypedBuilder, Clone, Debug, Serialize)]
#[serde(rename_all = "camelCase")]
pub struct Span {
pub(crate) struct Span {
#[builder(setter(strip_option), default)]
#[serde(skip_serializing_if = "Option::is_none")]
trace_id: Option<String>,
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-zipkin/src/uploader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use opentelemetry::exporter::trace;
static API_V2_COLLECTOR_ROUTE: &str = "/api/v2/spans";

#[derive(Clone, Debug)]
pub enum UploaderFormat {
pub(crate) enum UploaderFormat {
HTTP,
}

Expand Down
2 changes: 1 addition & 1 deletion scripts/lint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
set -eu

if rustup component add clippy; then
cargo clippy --all-targets --all -- \
cargo clippy --all-targets --all-features --workspace -- \
`# Exit with a nonzero code if there are clippy warnings` \
-Dwarnings \
"$@"
Expand Down
2 changes: 1 addition & 1 deletion src/api/metrics/measure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
//! durations and sizes.
//!
//! When passing `MetricOptions`, measures can be declared as
//! `with_abslute(false)` to indicate support for positive and negative values.
//! `with_absolute(false)` to indicate support for positive and negative values.
use crate::api::metrics;

/// An interface for recording values where the count or rate of
Expand Down
4 changes: 2 additions & 2 deletions src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ pub use metrics::{
value::MeasurementValue,
Instrument, InstrumentHandle, LabelSet, Measurement, Meter, MetricOptions,
};
pub use propagation::{binary_propagator::BinaryFormat, text_propagator::HttpTextFormat, Carrier};
#[cfg(feature="base64_format")]
#[cfg(feature = "base64_format")]
pub use propagation::base64_format::Base64Format;
pub use propagation::{binary_propagator::BinaryFormat, text_propagator::HttpTextFormat, Carrier};
pub use trace::{
b3_propagator::B3Propagator,
event::Event,
Expand Down
2 changes: 1 addition & 1 deletion src/api/trace/noop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ impl api::Span for NoopSpan {
// Ignored
}

/// Ignors name updates
/// Ignores name updates
fn update_name(&mut self, _new_name: String) {
// Ignored
}
Expand Down
3 changes: 2 additions & 1 deletion src/api/trace/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@
//! Implementations might require the user to specify configuration properties at
//! `Provider` creation time, or rely on external configuration.
use crate::api;
use std::fmt;

/// An interface to create `Tracer` instances.
pub trait Provider: Send + Sync {
pub trait Provider: fmt::Debug + 'static {
/// The `Tracer` type that this `Provider` will return.
type Tracer: api::Tracer;

Expand Down
3 changes: 2 additions & 1 deletion src/api/trace/span.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@
use crate::api;
#[cfg(feature = "serialize")]
use serde::{Deserialize, Serialize};
use std::fmt;
use std::time::SystemTime;

/// Interface for a single operation within a trace.
pub trait Span: Send + Sync + std::fmt::Debug {
pub trait Span: fmt::Debug + 'static {
/// An API to record events in the context of a given `Span`.
///
/// Events have a time associated with the moment when they are
Expand Down
3 changes: 2 additions & 1 deletion src/api/trace/tracer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@
//!
//! Docs: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/api-tracing.md#tracer
use crate::api::{self, Span};
use std::fmt;
use std::time::SystemTime;

/// Interface for constructing `Span`s.
pub trait Tracer: Send + Sync {
pub trait Tracer: fmt::Debug + 'static {
/// The `Span` type used by this `Tracer`.
type Span: api::Span;

Expand Down
Loading

0 comments on commit 2c687c5

Please sign in to comment.