Fix up some loose ends, review notes

apollo-router/src/plugins/telemetry/apollo_otlp_exporter.rs

@@ -1,8 +1,11 @@
+use std::borrow::Cow;
+
 use futures::future::BoxFuture;
+use itertools::Itertools;
 use sys_info::hostname;
 use tonic::metadata::MetadataMap;
 use tower::BoxError;
-use opentelemetry::{sdk::{export::trace::{ExportResult, SpanData, SpanExporter}, Resource}, InstrumentationLibrary, KeyValue};
+use opentelemetry::{sdk::{export::trace::{ExportResult, SpanData, SpanExporter}, trace::EvictedQueue, Resource}, trace::{SpanContext, Status, TraceFlags, TraceState}, InstrumentationLibrary, KeyValue};
 use url::Url;
 use uuid::Uuid;
 use crate::plugins::telemetry::{apollo_exporter::get_uname, metrics::apollo::ROUTER_ID, tracing::BatchProcessorConfig, GLOBAL_TRACER_NAME};
@@ -64,25 +67,31 @@ impl ApolloOtlpExporter {
       });
   }
 
-  pub(crate) fn submit_trace_batch(&self, traces: Vec<Vec<LightSpanData>>) -> BoxFuture<'static, ExportResult> {
+  pub(crate) async fn submit_trace_batch(&mut self, traces: Vec<Vec<LightSpanData>>) -> BoxFuture<'static, ExportResult> {
     let spans = traces.into_iter().flat_map(|t| {
       t.into_iter().map(|s| {
         SpanData {
-          span_context: None,
+          span_context: SpanContext::new(
+            s.trace_id,
+            s.span_id,
+            TraceFlags::default().with_sampled(true),
+            true,
+            TraceState::default(),
+          ),
           parent_span_id: s.parent_span_id,
           span_kind: s.span_kind,
           name: s.name,
           start_time: s.start_time,
           end_time: s.end_time,
           attributes: s.attributes,
-          events: None,
-          links: None,
-          status: s.status,
-          resource: self.resource_template, // Hooray, Cows!  Hopefully this is fine, it should never change.
-          instrumentation_lib: self.intrumentation_library,
+          events: EvictedQueue::new(0),
+          links: EvictedQueue::new(0),
+          status: Status::Unset,
+          resource: Cow::Owned(self.resource_template.to_owned()), // Hooray, Cows!  This might need a look.
+          instrumentation_lib: self.intrumentation_library.clone(),
         }
-      });
-    });
-    return self.otlp_exporter.export(spans.into());
+      })
+    }).collect_vec();
+    self.otlp_exporter.export(spans)
   }
 }

apollo-router/src/plugins/telemetry/tracing/apollo.rs

@@ -26,6 +26,10 @@ impl TracingConfigurator for Config {
         tracing::debug!("configuring Apollo tracing");
         let exporter = apollo_telemetry::Exporter::builder()
             .endpoint(&self.endpoint)
+            .otlp_endpoint(match self.experimental_tracing_protocol {
+                Apollo => None,
+                ApolloAndOtlp => Some(&self.experimental_otlp_endpoint),
+            })
             .apollo_key(
                 self.apollo_key
                     .as_ref()

apollo-router/src/plugins/telemetry/tracing/apollo_telemetry.rs

@@ -23,6 +23,7 @@ use opentelemetry::sdk::trace::EvictedHashMap;
 use opentelemetry::trace::SpanId;
 use opentelemetry::trace::SpanKind;
 use opentelemetry::trace::TraceError;
+use opentelemetry::trace::TraceId;
 use opentelemetry::Key;
 use opentelemetry::Value;
 use opentelemetry_semantic_conventions::trace::HTTP_REQUEST_METHOD;
@@ -141,6 +142,7 @@ pub(crate) enum Error {
 // Also, maybe we can just use the actual SpanData instead of the light one?
 #[derive(Debug, Clone, PartialEq)]
 pub(crate) struct LightSpanData {
+    pub trace_id: TraceId,
     pub span_id: SpanId,
     pub parent_span_id: SpanId,
     pub span_kind: SpanKind,
@@ -153,6 +155,7 @@ pub(crate) struct LightSpanData {
 impl From<SpanData> for LightSpanData {
     fn from(value: SpanData) -> Self {
         Self {
+            trace_id: value.span_context.trace_id(),
             span_id: value.span_context.span_id(),
             parent_span_id: value.parent_span_id,
             span_kind: value.span_kind,
@@ -360,13 +363,13 @@ impl Exporter {
             true => Vec::new(),
             false => vec![root_span],
         };
-        return spans_for_tree.append(child_spans);
+        spans_for_tree.append(child_spans)
     }
 
     fn group_by_trace(&mut self, span: LightSpanData) -> Vec<LightSpanData> {
         // TBD(tim): this could alternatively use the same algorithm in `groupbytrace` processor, which
         // groups based on trace ID instead of connecting recursively by parent ID.
-        return self.collect_spans_for_tree(&span);
+        self.collect_spans_for_tree(&span)
     }
 
     fn extract_data_from_spans(&mut self, span: &LightSpanData) -> Result<Vec<TreeData>, Error> {
@@ -878,7 +881,7 @@ impl SpanExporter for Exporter {
         if self.otlp_exporter.is_some() {
             let exporter = self.otlp_exporter.clone();
             let fut = async move {
-                exporter.export(batch)
+                exporter.export(otlp_trace_spans)
             };
         }
         fut.boxed()