Fix clippy warnings and deny them in CI (#453)

Author: djc

.github/workflows/ci.yml

@@ -23,20 +23,21 @@ jobs:
     - name: Test
       run: ./scripts/test.sh
   lint:
-    strategy:
-      matrix:
-        rust: [stable, beta]
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v1
       with:
         submodules: true
     - uses: actions-rs/toolchain@v1
       with:
-        toolchain: ${{ matrix.rust }}
+        toolchain: stable
         components: rustfmt
         profile: minimal
     - uses: arduino/setup-protoc@v1
+    - uses: actions-rs/cargo@v1
+      with:
+        command: fmt
+        args: --all -- --check
     - name: Lint
       run: ./scripts/lint.sh
   msrv:
@@ -55,7 +56,7 @@ jobs:
         cargo test --verbose --manifest-path=opentelemetry/Cargo.toml --features trace,metrics,serialize,tokio-support,serde,testing &&
         cargo test --manifest-path=opentelemetry-jaeger/Cargo.toml &&
         cargo test --manifest-path=opentelemetry-zipkin/Cargo.toml
-  meta:
+  coverage:
     continue-on-error: true
     runs-on: ubuntu-latest
     steps:
@@ -65,7 +66,6 @@ jobs:
     - uses: actions-rs/toolchain@v1
       with:
         toolchain: nightly
-        components: rustfmt, clippy
         override: true
     - uses: arduino/setup-protoc@v1
     - uses: actions-rs/cargo@v1
@@ -78,8 +78,3 @@ jobs:
         RUSTDOCFLAGS: '-Zprofile -Ccodegen-units=1 -Cinline-threshold=0 -Clink-dead-code -Coverflow-checks=off -Cpanic=abort -Zpanic_abort_tests'
     - uses: actions-rs/[email protected]
     - uses: codecov/codecov-action@v1
-    - run: cargo fmt -- --check
-    - uses: actions-rs/clippy-check@v1
-      with:
-        token: ${{ secrets.GITHUB_TOKEN }}
-        args: --all-features --workspace

opentelemetry-jaeger/src/exporter/mod.rs

@@ -26,7 +26,7 @@ use opentelemetry::{
     global, sdk,
     sdk::export::trace,
     trace::{Event, Link, SpanKind, StatusCode, TracerProvider},
-    Key, KeyValue, Value,
+    Key, KeyValue,
 };
 #[cfg(feature = "collector_client")]
 use opentelemetry_http::HttpClient;
@@ -86,15 +86,6 @@ pub struct Process {
     pub tags: Vec<KeyValue>,
 }
 
-impl Into<jaeger::Process> for Process {
-    fn into(self) -> jaeger::Process {
-        jaeger::Process::new(
-            self.service_name,
-            Some(self.tags.into_iter().map(Into::into).collect()),
-        )
-    }
-}
-
 #[async_trait]
 impl trace::SpanExporter for Exporter {
     /// Export spans to Jaeger
@@ -447,48 +438,6 @@ impl surf::middleware::Middleware for BasicAuthMiddleware {
     }
 }
 
-#[rustfmt::skip]
-impl Into<jaeger::Tag> for KeyValue {
-    fn into(self) -> jaeger::Tag {
-        let KeyValue { key, value } = self;
-        match value {
-            Value::String(s) => jaeger::Tag::new(key.into(), jaeger::TagType::String, Some(s.into()), None, None, None, None),
-            Value::F64(f) => jaeger::Tag::new(key.into(), jaeger::TagType::Double, None, Some(f.into()), None, None, None),
-            Value::Bool(b) => jaeger::Tag::new(key.into(), jaeger::TagType::Bool, None, None, Some(b), None, None),
-            Value::I64(i) => jaeger::Tag::new(key.into(), jaeger::TagType::Long, None, None, None, Some(i), None),
-            // TODO: better Array handling, jaeger thrift doesn't support arrays
-            v @ Value::Array(_) => jaeger::Tag::new(key.into(), jaeger::TagType::String, Some(v.to_string()), None, None, None, None),
-        }
-    }
-}
-
-impl Into<jaeger::Log> for Event {
-    fn into(self) -> jaeger::Log {
-        let timestamp = self
-            .timestamp
-            .duration_since(SystemTime::UNIX_EPOCH)
-            .unwrap_or_else(|_| Duration::from_secs(0))
-            .as_micros() as i64;
-        let mut event_set_via_attribute = false;
-        let mut fields = self
-            .attributes
-            .into_iter()
-            .map(|attr| {
-                if attr.key.as_str() == "event" {
-                    event_set_via_attribute = true;
-                };
-                attr.into()
-            })
-            .collect::<Vec<_>>();
-
-        if !event_set_via_attribute {
-            fields.push(Key::new("event").string(self.name).into());
-        }
-
-        jaeger::Log::new(timestamp, fields)
-    }
-}
-
 fn links_to_references(links: sdk::trace::EvictedQueue<Link>) -> Option<Vec<jaeger::SpanRef>> {
     if !links.is_empty() {
         let refs = links

opentelemetry-jaeger/src/exporter/thrift/mod.rs

@@ -1,6 +1,62 @@
 //! Thrift generated Jaeger client
 //!
 //! Definitions: https://github.com/uber/jaeger-idl/blob/master/thrift/
+use std::time::{Duration, SystemTime};
+
+use opentelemetry::trace::Event;
+use opentelemetry::{Key, KeyValue, Value};
+
 pub(crate) mod agent;
 pub(crate) mod jaeger;
 pub(crate) mod zipkincore;
+
+impl From<super::Process> for jaeger::Process {
+    fn from(process: super::Process) -> jaeger::Process {
+        jaeger::Process::new(
+            process.service_name,
+            Some(process.tags.into_iter().map(Into::into).collect()),
+        )
+    }
+}
+
+impl From<Event> for jaeger::Log {
+    fn from(event: crate::exporter::Event) -> jaeger::Log {
+        let timestamp = event
+            .timestamp
+            .duration_since(SystemTime::UNIX_EPOCH)
+            .unwrap_or_else(|_| Duration::from_secs(0))
+            .as_micros() as i64;
+        let mut event_set_via_attribute = false;
+        let mut fields = event
+            .attributes
+            .into_iter()
+            .map(|attr| {
+                if attr.key.as_str() == "event" {
+                    event_set_via_attribute = true;
+                };
+                attr.into()
+            })
+            .collect::<Vec<_>>();
+
+        if !event_set_via_attribute {
+            fields.push(Key::new("event").string(event.name).into());
+        }
+
+        jaeger::Log::new(timestamp, fields)
+    }
+}
+
+#[rustfmt::skip]
+impl From<KeyValue> for jaeger::Tag {
+    fn from(kv: KeyValue) -> jaeger::Tag {
+        let KeyValue { key, value } = kv;
+        match value {
+            Value::String(s) => jaeger::Tag::new(key.into(), jaeger::TagType::String, Some(s.into()), None, None, None, None),
+            Value::F64(f) => jaeger::Tag::new(key.into(), jaeger::TagType::Double, None, Some(f.into()), None, None, None),
+            Value::Bool(b) => jaeger::Tag::new(key.into(), jaeger::TagType::Bool, None, None, Some(b), None, None),
+            Value::I64(i) => jaeger::Tag::new(key.into(), jaeger::TagType::Long, None, None, None, Some(i), None),
+            // TODO: better Array handling, jaeger thrift doesn't support arrays
+            v @ Value::Array(_) => jaeger::Tag::new(key.into(), jaeger::TagType::String, Some(v.to_string()), None, None, None, None),
+        }
+    }
+}

opentelemetry-zipkin/src/exporter/model/annotation.rs

@@ -1,8 +1,11 @@
+use std::time::{Duration, SystemTime};
+
+use opentelemetry::trace::Event;
 use serde::Serialize;
 
 #[derive(TypedBuilder, Clone, Debug, Serialize)]
 #[serde(rename_all = "camelCase")]
-pub struct Annotation {
+pub(crate) struct Annotation {
     #[builder(setter(strip_option), default)]
     #[serde(skip_serializing_if = "Option::is_none")]
     timestamp: Option<u64>,
@@ -11,6 +14,22 @@ pub struct Annotation {
     value: Option<String>,
 }
 
+/// Converts `Event` into an `annotation::Annotation`
+impl From<Event> for Annotation {
+    fn from(event: Event) -> Annotation {
+        let timestamp = event
+            .timestamp
+            .duration_since(SystemTime::UNIX_EPOCH)
+            .unwrap_or_else(|_| Duration::from_secs(0))
+            .as_micros() as u64;
+
+        Annotation::builder()
+            .timestamp(timestamp)
+            .value(event.name)
+            .build()
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use crate::exporter::model::annotation::Annotation;

opentelemetry-zipkin/src/exporter/model/mod.rs

@@ -1,6 +1,6 @@
 use opentelemetry::{
     sdk::export::trace,
-    trace::{Event, SpanKind, StatusCode},
+    trace::{SpanKind, StatusCode},
     Key, KeyValue,
 };
 use std::collections::HashMap;
@@ -17,22 +17,6 @@ const INSTRUMENTATION_LIBRARY_VERSION: &str = "otel.library.version";
 const OTEL_ERROR_DESCRIPTION: &str = "error";
 const OTEL_STATUS_CODE: &str = "otel.status_code";
 
-/// Converts `Event` into an `annotation::Annotation`
-impl Into<annotation::Annotation> for Event {
-    fn into(self) -> annotation::Annotation {
-        let timestamp = self
-            .timestamp
-            .duration_since(SystemTime::UNIX_EPOCH)
-            .unwrap_or_else(|_| Duration::from_secs(0))
-            .as_micros() as u64;
-
-        annotation::Annotation::builder()
-            .timestamp(timestamp)
-            .value(self.name)
-            .build()
-    }
-}
-
 /// Converts StatusCode to Option<&'static str>
 /// `Unset` status code is unused.
 fn from_statuscode_to_str(status_code: StatusCode) -> Option<&'static str> {

opentelemetry/src/trace/span_context.rs

@@ -302,7 +302,7 @@ impl FromStr for TraceState {
             }
         }
 
-        Ok(TraceState::from_key_value(key_value_pairs)?)
+        TraceState::from_key_value(key_value_pairs)
     }
 }