From 92fae737f7be6c83e13e6378e6b9076b251cc636 Mon Sep 17 00:00:00 2001 From: Josh W Lewis Date: Thu, 6 Feb 2025 10:42:10 -0600 Subject: [PATCH 01/12] Implement FileExporter --- libcnb/Cargo.toml | 9 ++--- libcnb/src/tracing.rs | 90 +++++++++++++++++++++++++++++++++---------- 2 files changed, 73 insertions(+), 26 deletions(-) diff --git a/libcnb/Cargo.toml b/libcnb/Cargo.toml index c61ee392..00435685 100644 --- a/libcnb/Cargo.toml +++ b/libcnb/Cargo.toml @@ -18,7 +18,7 @@ workspace = true trace = [ "dep:opentelemetry", "dep:opentelemetry_sdk", - "dep:opentelemetry-stdout", + "dep:opentelemetry_proto ", ] [dependencies] @@ -27,15 +27,14 @@ cyclonedx-bom = { version = "0.8.0", optional = true } libcnb-common.workspace = true libcnb-data.workspace = true libcnb-proc-macros.workspace = true +futures-core = "0.3" opentelemetry = { version = "0.24", optional = true } opentelemetry_sdk = { version = "0.24", optional = true } -opentelemetry-stdout = { version = "0.5", optional = true, features = [ - "trace", -] } +opentelemetry-proto = { version = "0.7", optional = true } serde = { version = "1.0.215", features = ["derive"] } +serde_json = "1.0.133" thiserror = "2.0.6" toml.workspace = true [dev-dependencies] -serde_json = "1.0.133" tempfile = "3.14.0" diff --git a/libcnb/src/tracing.rs b/libcnb/src/tracing.rs index 2eb40e61..1de54469 100644 --- a/libcnb/src/tracing.rs +++ b/libcnb/src/tracing.rs @@ -1,14 +1,24 @@ +use futures_core::future::BoxFuture; use libcnb_data::buildpack::Buildpack; use opentelemetry::{ global, - trace::{Span as SpanTrait, Status, Tracer, TracerProvider as TracerProviderTrait}, + trace::{Span as SpanTrait, Status, TraceError, Tracer, TracerProvider as TracerProviderTrait}, KeyValue, }; +use opentelemetry_proto::transform::common::tonic::ResourceAttributesWithSchema; +use opentelemetry_proto::transform::trace::tonic::group_spans_by_resource_and_scope; use opentelemetry_sdk::{ + export::trace::SpanExporter, trace::{Config, Span, TracerProvider}, Resource, }; -use std::{io::BufWriter, path::Path}; +use std::{ + fmt::Debug, + fs::{File, OpenOptions}, + io::{BufWriter, Error, LineWriter, Write}, + path::{Path, PathBuf}, + sync::{Arc, Mutex}, +}; // This is the directory in which `BuildpackTrace` stores OpenTelemetry File // Exports. Services which intend to export the tracing data from libcnb.rs @@ -40,34 +50,31 @@ pub(crate) fn start_trace(buildpack: &Buildpack, phase_name: &'static str) -> Bu if let Some(parent_dir) = tracing_file_path.parent() { let _ = std::fs::create_dir_all(parent_dir); } - let exporter = match std::fs::File::options() + + let provider_builder = + TracerProvider::builder().with_config(Config::default().with_resource(Resource::new([ + // Associate the tracer provider with service attributes. The buildpack + // name/version seems to map well to the suggestion here + // https://opentelemetry.io/docs/specs/semconv/resource/#service. + KeyValue::new("service.name", buildpack.id.to_string()), + KeyValue::new("service.version", buildpack.version.to_string()), + ]))); + + let provider = match std::fs::File::options() .create(true) .append(true) .open(&tracing_file_path) + .map(FileExporter::new) { // Write tracing data to a file, which may be read by other // services. Wrap with a BufWriter to prevent serde from sending each // JSON token to IO, and instead send entire JSON objects to IO. - Ok(file) => opentelemetry_stdout::SpanExporter::builder() - .with_writer(BufWriter::new(file)) - .build(), + Ok(exporter) => provider_builder.with_simple_exporter(exporter), // Failed tracing shouldn't fail a build, and any logging here would // likely confuse the user, so send telemetry to /dev/null on errors. - Err(_) => opentelemetry_stdout::SpanExporter::builder() - .with_writer(std::io::sink()) - .build(), - }; - - let provider = TracerProvider::builder() - .with_simple_exporter(exporter) - .with_config(Config::default().with_resource(Resource::new([ - // Associate the tracer provider with service attributes. The buildpack - // name/version seems to map well to the suggestion here - // https://opentelemetry.io/docs/specs/semconv/resource/#service. - KeyValue::new("service.name", buildpack.id.to_string()), - KeyValue::new("service.version", buildpack.version.to_string()), - ]))) - .build(); + Err(_) => provider_builder, + } + .build(); // Set the global tracer provider so that buildpacks may use it. global::set_tracer_provider(provider.clone()); @@ -114,6 +121,47 @@ impl Drop for BuildpackTrace { } } +#[derive(Debug)] +struct FileExporter { + w: Arc>, +} + +impl FileExporter { + fn new(w: W) -> Self { + Self { + w: Arc::new(Mutex::new(w)), + } + } +} + +impl SpanExporter for FileExporter { + fn export( + &mut self, + batch: Vec, + ) -> BoxFuture<'static, opentelemetry_sdk::export::trace::ExportResult> { + let resource = ResourceAttributesWithSchema::default(); + + let data = group_spans_by_resource_and_scope(batch, &resource); + let json = serde_json::to_string(&data); + let line = match json { + Ok(line) => line, + Err(e) => { + return Box::pin(std::future::ready(Err(TraceError::from(e.to_string())))); + } + }; + let mut file = match self.w.lock() { + Ok(f) => f, + Err(e) => { + return Box::pin(std::future::ready(Err(TraceError::from(e.to_string())))); + } + }; + match file.write_all(line.as_bytes()) { + Ok(()) => Box::pin(std::future::ready(Ok(()))), + Err(e) => Box::pin(std::future::ready(Err(TraceError::from(e.to_string())))), + } + } +} + #[cfg(test)] mod tests { use super::start_trace; From da85a8f580345a608c33d4dea880df622a1d15fa Mon Sep 17 00:00:00 2001 From: Josh W Lewis Date: Thu, 6 Feb 2025 17:11:20 -0600 Subject: [PATCH 02/12] Update opentelemetry and add resource to BuildpackTrace --- libcnb/Cargo.toml | 11 ++++---- libcnb/src/tracing.rs | 60 ++++++++++++++++++++++--------------------- 2 files changed, 37 insertions(+), 34 deletions(-) diff --git a/libcnb/Cargo.toml b/libcnb/Cargo.toml index 00435685..556d766c 100644 --- a/libcnb/Cargo.toml +++ b/libcnb/Cargo.toml @@ -18,7 +18,8 @@ workspace = true trace = [ "dep:opentelemetry", "dep:opentelemetry_sdk", - "dep:opentelemetry_proto ", + # "dep:opentelemetry_proto ", + "dep:serde_json", ] [dependencies] @@ -28,11 +29,11 @@ libcnb-common.workspace = true libcnb-data.workspace = true libcnb-proc-macros.workspace = true futures-core = "0.3" -opentelemetry = { version = "0.24", optional = true } -opentelemetry_sdk = { version = "0.24", optional = true } -opentelemetry-proto = { version = "0.7", optional = true } +opentelemetry = { version = "0.27.0", optional = true } +opentelemetry_sdk = { version = "0.27.0", optional = true } +opentelemetry-proto = { version = "0.27.0", optional = false } serde = { version = "1.0.215", features = ["derive"] } -serde_json = "1.0.133" +serde_json = { version = "1.0.133", optional = true } thiserror = "2.0.6" toml.workspace = true diff --git a/libcnb/src/tracing.rs b/libcnb/src/tracing.rs index 1de54469..de302d84 100644 --- a/libcnb/src/tracing.rs +++ b/libcnb/src/tracing.rs @@ -1,22 +1,21 @@ use futures_core::future::BoxFuture; use libcnb_data::buildpack::Buildpack; use opentelemetry::{ - global, + global::{self, BoxedSpan}, trace::{Span as SpanTrait, Status, TraceError, Tracer, TracerProvider as TracerProviderTrait}, - KeyValue, + InstrumentationScope, KeyValue, }; use opentelemetry_proto::transform::common::tonic::ResourceAttributesWithSchema; use opentelemetry_proto::transform::trace::tonic::group_spans_by_resource_and_scope; use opentelemetry_sdk::{ export::trace::SpanExporter, - trace::{Config, Span, TracerProvider}, + trace::TracerProvider, Resource, }; use std::{ fmt::Debug, - fs::{File, OpenOptions}, - io::{BufWriter, Error, LineWriter, Write}, - path::{Path, PathBuf}, + io::Write, + path::Path, sync::{Arc, Mutex}, }; @@ -33,7 +32,7 @@ const TELEMETRY_EXPORT_ROOT: &str = "/tmp/libcnb-telemetry"; /// a single CNB build or detect phase. pub(crate) struct BuildpackTrace { provider: TracerProvider, - span: Span, + span: BoxedSpan, } /// Start an OpenTelemetry trace and span that exports to an @@ -51,27 +50,28 @@ pub(crate) fn start_trace(buildpack: &Buildpack, phase_name: &'static str) -> Bu let _ = std::fs::create_dir_all(parent_dir); } - let provider_builder = - TracerProvider::builder().with_config(Config::default().with_resource(Resource::new([ - // Associate the tracer provider with service attributes. The buildpack - // name/version seems to map well to the suggestion here - // https://opentelemetry.io/docs/specs/semconv/resource/#service. - KeyValue::new("service.name", buildpack.id.to_string()), - KeyValue::new("service.version", buildpack.version.to_string()), - ]))); + let resource = Resource::new([ + // Define a resource that defines the trace provider. + // The buildpac name/version seems to map well to the suggestion here + // https://opentelemetry.io/docs/specs/semconv/resource/#service. + KeyValue::new("service.name", buildpack.id.to_string()), + KeyValue::new("service.version", buildpack.version.to_string()), + ]); + + let provider_builder = TracerProvider::builder().with_resource(resource.clone()); let provider = match std::fs::File::options() .create(true) .append(true) .open(&tracing_file_path) - .map(FileExporter::new) + .map(|file| FileExporter::new(file, resource)) { // Write tracing data to a file, which may be read by other // services. Wrap with a BufWriter to prevent serde from sending each // JSON token to IO, and instead send entire JSON objects to IO. Ok(exporter) => provider_builder.with_simple_exporter(exporter), - // Failed tracing shouldn't fail a build, and any logging here would - // likely confuse the user, so send telemetry to /dev/null on errors. + // Failed tracing shouldn't fail a build, and any export logging here + // would likely confuse the user, so we won't export when the file has IO errors Err(_) => provider_builder, } .build(); @@ -82,10 +82,11 @@ pub(crate) fn start_trace(buildpack: &Buildpack, phase_name: &'static str) -> Bu // Get a tracer identified by the instrumentation scope/library. The libcnb // crate name/version seems to map well to the suggestion here: // https://opentelemetry.io/docs/specs/otel/trace/api/#get-a-tracer. - let tracer = provider - .tracer_builder(env!("CARGO_PKG_NAME")) - .with_version(env!("CARGO_PKG_VERSION")) - .build(); + let tracer = global::tracer_provider().tracer_with_scope( + InstrumentationScope::builder(env!("CARGO_PKG_NAME")) + .with_version(env!("CARGO_PKG_VERSION")) + .build(), + ); let mut span = tracer.start(trace_name); span.set_attributes([ @@ -117,19 +118,21 @@ impl Drop for BuildpackTrace { fn drop(&mut self) { self.span.end(); self.provider.force_flush(); - global::shutdown_tracer_provider(); + self.provider.shutdown().ok(); } } #[derive(Debug)] struct FileExporter { - w: Arc>, + resource: Resource, + writer: Arc>, } impl FileExporter { - fn new(w: W) -> Self { + fn new(w: W, r: Resource) -> Self { Self { - w: Arc::new(Mutex::new(w)), + resource: r, + writer: Arc::new(Mutex::new(w)), } } } @@ -139,8 +142,7 @@ impl SpanExporter for FileExporter { &mut self, batch: Vec, ) -> BoxFuture<'static, opentelemetry_sdk::export::trace::ExportResult> { - let resource = ResourceAttributesWithSchema::default(); - + let resource = ResourceAttributesWithSchema::from(&self.resource); let data = group_spans_by_resource_and_scope(batch, &resource); let json = serde_json::to_string(&data); let line = match json { @@ -149,7 +151,7 @@ impl SpanExporter for FileExporter { return Box::pin(std::future::ready(Err(TraceError::from(e.to_string())))); } }; - let mut file = match self.w.lock() { + let mut file = match self.writer.lock() { Ok(f) => f, Err(e) => { return Box::pin(std::future::ready(Err(TraceError::from(e.to_string())))); From 6fef47090d46ee682e6de3cbda5a9d9bd47ff6c6 Mon Sep 17 00:00:00 2001 From: Josh W Lewis Date: Thu, 6 Feb 2025 17:26:59 -0600 Subject: [PATCH 03/12] Make opentelemetry-proto optional --- libcnb/Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libcnb/Cargo.toml b/libcnb/Cargo.toml index 556d766c..171d7c43 100644 --- a/libcnb/Cargo.toml +++ b/libcnb/Cargo.toml @@ -18,7 +18,7 @@ workspace = true trace = [ "dep:opentelemetry", "dep:opentelemetry_sdk", - # "dep:opentelemetry_proto ", + "dep:opentelemetry-proto ", "dep:serde_json", ] @@ -31,7 +31,7 @@ libcnb-proc-macros.workspace = true futures-core = "0.3" opentelemetry = { version = "0.27.0", optional = true } opentelemetry_sdk = { version = "0.27.0", optional = true } -opentelemetry-proto = { version = "0.27.0", optional = false } +opentelemetry-proto = { version = "0.27.0", optional = true } serde = { version = "1.0.215", features = ["derive"] } serde_json = { version = "1.0.133", optional = true } thiserror = "2.0.6" From f36fbd5abbab643e89af25bc5b7c630c23112f73 Mon Sep 17 00:00:00 2001 From: Josh W Lewis Date: Thu, 6 Feb 2025 18:57:53 -0600 Subject: [PATCH 04/12] Correct Cargo.toml syntax --- libcnb/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcnb/Cargo.toml b/libcnb/Cargo.toml index 171d7c43..156a8750 100644 --- a/libcnb/Cargo.toml +++ b/libcnb/Cargo.toml @@ -18,7 +18,7 @@ workspace = true trace = [ "dep:opentelemetry", "dep:opentelemetry_sdk", - "dep:opentelemetry-proto ", + "dep:opentelemetry-proto", "dep:serde_json", ] From 3b8baeccea536a47d35f7c71283b27ccb31267aa Mon Sep 17 00:00:00 2001 From: Josh W Lewis Date: Mon, 10 Feb 2025 13:34:27 -0600 Subject: [PATCH 05/12] Serialize direct to writer; skip the string --- libcnb/src/tracing.rs | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/libcnb/src/tracing.rs b/libcnb/src/tracing.rs index de302d84..0c9e6342 100644 --- a/libcnb/src/tracing.rs +++ b/libcnb/src/tracing.rs @@ -7,14 +7,10 @@ use opentelemetry::{ }; use opentelemetry_proto::transform::common::tonic::ResourceAttributesWithSchema; use opentelemetry_proto::transform::trace::tonic::group_spans_by_resource_and_scope; -use opentelemetry_sdk::{ - export::trace::SpanExporter, - trace::TracerProvider, - Resource, -}; +use opentelemetry_sdk::{export::trace::SpanExporter, trace::TracerProvider, Resource}; use std::{ fmt::Debug, - io::Write, + io::{LineWriter, Write}, path::Path, sync::{Arc, Mutex}, }; @@ -124,15 +120,15 @@ impl Drop for BuildpackTrace { #[derive(Debug)] struct FileExporter { + writer: Arc>>, resource: Resource, - writer: Arc>, } impl FileExporter { - fn new(w: W, r: Resource) -> Self { + fn new(writer: W, resource: Resource) -> Self { Self { - resource: r, - writer: Arc::new(Mutex::new(w)), + writer: Arc::new(Mutex::new(LineWriter::new(writer))), + resource, } } } @@ -144,20 +140,19 @@ impl SpanExporter for FileExporter { ) -> BoxFuture<'static, opentelemetry_sdk::export::trace::ExportResult> { let resource = ResourceAttributesWithSchema::from(&self.resource); let data = group_spans_by_resource_and_scope(batch, &resource); - let json = serde_json::to_string(&data); - let line = match json { - Ok(line) => line, + let mut writer = match self.writer.lock() { + Ok(f) => f, Err(e) => { return Box::pin(std::future::ready(Err(TraceError::from(e.to_string())))); } }; - let mut file = match self.writer.lock() { - Ok(f) => f, + match serde_json::to_writer(writer.get_mut(), &data) { + Ok(()) => (), Err(e) => { return Box::pin(std::future::ready(Err(TraceError::from(e.to_string())))); } }; - match file.write_all(line.as_bytes()) { + match writer.flush() { Ok(()) => Box::pin(std::future::ready(Ok(()))), Err(e) => Box::pin(std::future::ready(Err(TraceError::from(e.to_string())))), } From fb49f966b70891a8e082343c98f07c7d4749e4fe Mon Sep 17 00:00:00 2001 From: Josh W Lewis Date: Mon, 10 Feb 2025 15:06:35 -0600 Subject: [PATCH 06/12] Conditional compilation for trace dependencies --- libcnb/Cargo.toml | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/libcnb/Cargo.toml b/libcnb/Cargo.toml index 156a8750..493b87b5 100644 --- a/libcnb/Cargo.toml +++ b/libcnb/Cargo.toml @@ -16,9 +16,15 @@ workspace = true [features] trace = [ + "dep:futures-core", "dep:opentelemetry", + "opentelemetry/trace", "dep:opentelemetry_sdk", + "opentelemetry_sdk/trace", "dep:opentelemetry-proto", + "opentelemetry-proto/trace", + "opentelemetry-proto/gen-tonic-messages", + "opentelemetry-proto/with-serde", "dep:serde_json", ] @@ -28,10 +34,10 @@ cyclonedx-bom = { version = "0.8.0", optional = true } libcnb-common.workspace = true libcnb-data.workspace = true libcnb-proc-macros.workspace = true -futures-core = "0.3" -opentelemetry = { version = "0.27.0", optional = true } -opentelemetry_sdk = { version = "0.27.0", optional = true } -opentelemetry-proto = { version = "0.27.0", optional = true } +futures-core = { version = "0.3", optional = true } +opentelemetry = { version = "0.27.0", optional = true, default-features = false } +opentelemetry_sdk = { version = "0.27.0", optional = true, default-features = false } +opentelemetry-proto = { version = "0.27.0", optional = true, default-features = false } serde = { version = "1.0.215", features = ["derive"] } serde_json = { version = "1.0.133", optional = true } thiserror = "2.0.6" From 7509de546e9b5190eeca128609c19fd4bc918cb5 Mon Sep 17 00:00:00 2001 From: Josh W Lewis Date: Mon, 10 Feb 2025 17:42:16 -0600 Subject: [PATCH 07/12] Update to opentelemetry 0.28 --- libcnb/Cargo.toml | 6 ++--- libcnb/src/tracing.rs | 55 ++++++++++++++++++++++++++++++------------- 2 files changed, 42 insertions(+), 19 deletions(-) diff --git a/libcnb/Cargo.toml b/libcnb/Cargo.toml index 493b87b5..72b7b473 100644 --- a/libcnb/Cargo.toml +++ b/libcnb/Cargo.toml @@ -35,9 +35,9 @@ libcnb-common.workspace = true libcnb-data.workspace = true libcnb-proc-macros.workspace = true futures-core = { version = "0.3", optional = true } -opentelemetry = { version = "0.27.0", optional = true, default-features = false } -opentelemetry_sdk = { version = "0.27.0", optional = true, default-features = false } -opentelemetry-proto = { version = "0.27.0", optional = true, default-features = false } +opentelemetry = { version = "0.28.0", optional = true, default-features = false } +opentelemetry_sdk = { version = "0.28.0", optional = true, default-features = false } +opentelemetry-proto = { version = "0.28.0", optional = true, default-features = false } serde = { version = "1.0.215", features = ["derive"] } serde_json = { version = "1.0.133", optional = true } thiserror = "2.0.6" diff --git a/libcnb/src/tracing.rs b/libcnb/src/tracing.rs index 0c9e6342..b5c366cf 100644 --- a/libcnb/src/tracing.rs +++ b/libcnb/src/tracing.rs @@ -2,12 +2,17 @@ use futures_core::future::BoxFuture; use libcnb_data::buildpack::Buildpack; use opentelemetry::{ global::{self, BoxedSpan}, - trace::{Span as SpanTrait, Status, TraceError, Tracer, TracerProvider as TracerProviderTrait}, + trace::{Span as SpanTrait, Status, Tracer, TracerProvider as TracerProviderTrait}, InstrumentationScope, KeyValue, }; use opentelemetry_proto::transform::common::tonic::ResourceAttributesWithSchema; use opentelemetry_proto::transform::trace::tonic::group_spans_by_resource_and_scope; -use opentelemetry_sdk::{export::trace::SpanExporter, trace::TracerProvider, Resource}; +use opentelemetry_sdk::{ + error::{OTelSdkError, OTelSdkResult}, + trace::SdkTracerProvider, + trace::SpanExporter, + Resource, +}; use std::{ fmt::Debug, io::{LineWriter, Write}, @@ -27,7 +32,7 @@ const TELEMETRY_EXPORT_ROOT: &str = "/tmp/libcnb-telemetry"; /// Represents an OpenTelemetry tracer provider and single span tracing /// a single CNB build or detect phase. pub(crate) struct BuildpackTrace { - provider: TracerProvider, + provider: SdkTracerProvider, span: BoxedSpan, } @@ -46,15 +51,17 @@ pub(crate) fn start_trace(buildpack: &Buildpack, phase_name: &'static str) -> Bu let _ = std::fs::create_dir_all(parent_dir); } - let resource = Resource::new([ + let resource = Resource::builder() // Define a resource that defines the trace provider. // The buildpac name/version seems to map well to the suggestion here // https://opentelemetry.io/docs/specs/semconv/resource/#service. - KeyValue::new("service.name", buildpack.id.to_string()), - KeyValue::new("service.version", buildpack.version.to_string()), - ]); + .with_attributes([ + KeyValue::new("service.name", buildpack.id.to_string()), + KeyValue::new("service.version", buildpack.version.to_string()), + ]) + .build(); - let provider_builder = TracerProvider::builder().with_resource(resource.clone()); + let provider_builder = SdkTracerProvider::builder().with_resource(resource.clone()); let provider = match std::fs::File::options() .create(true) @@ -113,7 +120,7 @@ impl BuildpackTrace { impl Drop for BuildpackTrace { fn drop(&mut self) { self.span.end(); - self.provider.force_flush(); + self.provider.force_flush().ok(); self.provider.shutdown().ok(); } } @@ -136,27 +143,43 @@ impl FileExporter { impl SpanExporter for FileExporter { fn export( &mut self, - batch: Vec, - ) -> BoxFuture<'static, opentelemetry_sdk::export::trace::ExportResult> { + batch: Vec, + ) -> BoxFuture<'static, OTelSdkResult> { let resource = ResourceAttributesWithSchema::from(&self.resource); let data = group_spans_by_resource_and_scope(batch, &resource); let mut writer = match self.writer.lock() { Ok(f) => f, Err(e) => { - return Box::pin(std::future::ready(Err(TraceError::from(e.to_string())))); + return Box::pin(std::future::ready(Err(OTelSdkError::InternalFailure( + e.to_string(), + )))); } }; match serde_json::to_writer(writer.get_mut(), &data) { - Ok(()) => (), + Ok(()) => Box::pin(std::future::ready(Ok(()))), + Err(e) => Box::pin(std::future::ready(Err(OTelSdkError::InternalFailure( + e.to_string(), + )))), + } + } + + fn force_flush(&mut self) -> OTelSdkResult { + let mut writer = match self.writer.lock() { + Ok(f) => f, Err(e) => { - return Box::pin(std::future::ready(Err(TraceError::from(e.to_string())))); + return Err(OTelSdkError::InternalFailure(e.to_string())); } }; + match writer.flush() { - Ok(()) => Box::pin(std::future::ready(Ok(()))), - Err(e) => Box::pin(std::future::ready(Err(TraceError::from(e.to_string())))), + Ok(()) => Ok(()), + Err(e) => Err(OTelSdkError::InternalFailure(e.to_string())), } } + + fn set_resource(&mut self, res: &opentelemetry_sdk::Resource) { + self.resource = res.clone(); + } } #[cfg(test)] From 7da1c4e4b75e6d3181e01c41bb7067b8d1e4b956 Mon Sep 17 00:00:00 2001 From: Josh W Lewis Date: Wed, 12 Feb 2025 14:03:59 -0600 Subject: [PATCH 08/12] Use the batch exporter --- libcnb/src/tracing.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/libcnb/src/tracing.rs b/libcnb/src/tracing.rs index b5c366cf..1d5f7b40 100644 --- a/libcnb/src/tracing.rs +++ b/libcnb/src/tracing.rs @@ -53,7 +53,7 @@ pub(crate) fn start_trace(buildpack: &Buildpack, phase_name: &'static str) -> Bu let resource = Resource::builder() // Define a resource that defines the trace provider. - // The buildpac name/version seems to map well to the suggestion here + // The buildpack name/version seems to map well to the suggestion here // https://opentelemetry.io/docs/specs/semconv/resource/#service. .with_attributes([ KeyValue::new("service.name", buildpack.id.to_string()), @@ -69,12 +69,10 @@ pub(crate) fn start_trace(buildpack: &Buildpack, phase_name: &'static str) -> Bu .open(&tracing_file_path) .map(|file| FileExporter::new(file, resource)) { - // Write tracing data to a file, which may be read by other - // services. Wrap with a BufWriter to prevent serde from sending each - // JSON token to IO, and instead send entire JSON objects to IO. - Ok(exporter) => provider_builder.with_simple_exporter(exporter), + // Write tracing data to a file, which may be read by other services + Ok(exporter) => provider_builder.with_batch_exporter(exporter), // Failed tracing shouldn't fail a build, and any export logging here - // would likely confuse the user, so we won't export when the file has IO errors + // would likely confuse the user; don't export when the file has IO errors Err(_) => provider_builder, } .build(); From 9070f11c4cf28e9f8407df13831f509186fd719e Mon Sep 17 00:00:00 2001 From: Josh W Lewis Date: Wed, 12 Feb 2025 17:55:11 -0600 Subject: [PATCH 09/12] Simplify FileExporter matches --- libcnb/src/tracing.rs | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/libcnb/src/tracing.rs b/libcnb/src/tracing.rs index 1d5f7b40..47e7d43c 100644 --- a/libcnb/src/tracing.rs +++ b/libcnb/src/tracing.rs @@ -153,26 +153,21 @@ impl SpanExporter for FileExporter { )))); } }; - match serde_json::to_writer(writer.get_mut(), &data) { - Ok(()) => Box::pin(std::future::ready(Ok(()))), - Err(e) => Box::pin(std::future::ready(Err(OTelSdkError::InternalFailure( - e.to_string(), - )))), - } + Box::pin(std::future::ready( + serde_json::to_writer(writer.get_mut(), &data) + .map_err(|e| OTelSdkError::InternalFailure(e.to_string())), + )) } fn force_flush(&mut self) -> OTelSdkResult { - let mut writer = match self.writer.lock() { - Ok(f) => f, - Err(e) => { - return Err(OTelSdkError::InternalFailure(e.to_string())); - } - }; - - match writer.flush() { - Ok(()) => Ok(()), - Err(e) => Err(OTelSdkError::InternalFailure(e.to_string())), - } + let mut writer = self + .writer + .lock() + .map_err(|e| OTelSdkError::InternalFailure(e.to_string()))?; + + writer + .flush() + .map_err(|e| OTelSdkError::InternalFailure(e.to_string())) } fn set_resource(&mut self, res: &opentelemetry_sdk::Resource) { From f866735666dba5f6c1d43d40079cca7bce2dd57e Mon Sep 17 00:00:00 2001 From: Josh W Lewis Date: Wed, 12 Feb 2025 17:55:44 -0600 Subject: [PATCH 10/12] Ensure serde_json is also in dev-dependencies for testing --- libcnb/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/libcnb/Cargo.toml b/libcnb/Cargo.toml index 72b7b473..a98fb333 100644 --- a/libcnb/Cargo.toml +++ b/libcnb/Cargo.toml @@ -45,3 +45,4 @@ toml.workspace = true [dev-dependencies] tempfile = "3.14.0" +serde_json = "1.0.133" From 940cd421e6081e9443fded4728df48170c814849 Mon Sep 17 00:00:00 2001 From: Josh W Lewis Date: Wed, 12 Feb 2025 18:01:26 -0600 Subject: [PATCH 11/12] Add changelog entry for OTLP File Exporter --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a64f11a..b6f398d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Implemented custom OTLP File Exporter instead of `opentelemetry-stdout` and updated `opentelemetry` libraries to `0.28`. ([#909](https://github.com/heroku/libcnb.rs/pull/909/)) ## [0.26.1] - 2024-12-10 From 3d4a02d8aa6b7a0d40592d42f7c9455506aa5e88 Mon Sep 17 00:00:00 2001 From: Josh W Lewis Date: Wed, 12 Feb 2025 18:02:31 -0600 Subject: [PATCH 12/12] Update changelog with crate in question --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6f398d3..af53fd21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- Implemented custom OTLP File Exporter instead of `opentelemetry-stdout` and updated `opentelemetry` libraries to `0.28`. ([#909](https://github.com/heroku/libcnb.rs/pull/909/)) +- `libcnb`: + - Implemented custom OTLP File Exporter instead of `opentelemetry-stdout` and updated `opentelemetry` libraries to `0.28`. ([#909](https://github.com/heroku/libcnb.rs/pull/909/)) ## [0.26.1] - 2024-12-10