From 83f65caffc3a4155b805d02b42dc6fb5c3487fb1 Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Mon, 30 Dec 2024 22:13:24 +0000 Subject: [PATCH 1/6] Update LogBatch --- opentelemetry-sdk/src/export/logs/mod.rs | 67 ++++++++++++++++++--- opentelemetry-sdk/src/logs/log_processor.rs | 7 +-- 2 files changed, 61 insertions(+), 13 deletions(-) diff --git a/opentelemetry-sdk/src/export/logs/mod.rs b/opentelemetry-sdk/src/export/logs/mod.rs index 902adb54dd..ac29e1ad16 100644 --- a/opentelemetry-sdk/src/export/logs/mod.rs +++ b/opentelemetry-sdk/src/export/logs/mod.rs @@ -18,9 +18,18 @@ use std::fmt::Debug; /// #[derive(Debug)] pub struct LogBatch<'a> { - /// The data field contains a slice of tuples, where each tuple consists of a reference to - /// a `LogRecord` and a reference to an `InstrumentationScope`. - data: &'a [(&'a LogRecord, &'a InstrumentationScope)], + data: LogBatchData<'a>, +} + +/// The `LogBatchData` enum represents the data field of a `LogBatch`. +/// It can either be: +/// - A mutable reference to a vector of tuples, where each tuple consists of a `LogRecord` and an `InstrumentationScope`. +/// - Or it can be a slice of tuples, where each tuple consists of a reference to a `LogRecord` and a reference to an `InstrumentationScope`. +#[derive(Debug)] +#[allow(clippy::vec_box)] // TODO: Revisit this. Clippy complains about using Box in a Vec, but it's done here for performant moves of the data between channel and the vec. +enum LogBatchData<'a> { + BorrowedVec(&'a mut Vec>), // Used by BatchProcessor which clones the LogRecords for its own use. + BorrowedSlice(&'a [(&'a LogRecord, &'a InstrumentationScope)]), } impl<'a> LogBatch<'a> { @@ -39,7 +48,18 @@ impl<'a> LogBatch<'a> { /// Note - this is not a public function, and should not be used directly. This would be /// made private in the future. pub fn new(data: &'a [(&'a LogRecord, &'a InstrumentationScope)]) -> LogBatch<'a> { - LogBatch { data } + LogBatch { + data: LogBatchData::BorrowedSlice(data), + } + } + + #[allow(clippy::vec_box)] // TODO: Revisit this. Clippy complains about using Box in a Vec, but it's done here for performant moves of the data between channel and the vec. + pub(crate) fn new_with_owned_data( + data: &'a mut Vec>, + ) -> LogBatch<'a> { + LogBatch { + data: LogBatchData::BorrowedVec(data), + } } } @@ -54,9 +74,42 @@ impl LogBatch<'_> { /// An iterator that yields references to the `LogRecord` and `InstrumentationScope` in the batch. /// pub fn iter(&self) -> impl Iterator { - self.data - .iter() - .map(|(record, library)| (*record, *library)) + LogBatchDataIter { + data: &self.data, + index: 0, + } + } +} + +struct LogBatchDataIter<'a> { + data: &'a LogBatchData<'a>, + index: usize, +} + +impl<'a> Iterator for LogBatchDataIter<'a> { + type Item = (&'a LogRecord, &'a InstrumentationScope); + + fn next(&mut self) -> Option { + match self.data { + LogBatchData::BorrowedVec(data) => { + if self.index < data.len() { + let record = &*data[self.index]; + self.index += 1; + Some((&record.0, &record.1)) + } else { + None + } + } + LogBatchData::BorrowedSlice(data) => { + if self.index < data.len() { + let record = &data[self.index]; + self.index += 1; + Some((record.0, record.1)) + } else { + None + } + } + } } } diff --git a/opentelemetry-sdk/src/logs/log_processor.rs b/opentelemetry-sdk/src/logs/log_processor.rs index 61823ab854..b5fccd51f8 100644 --- a/opentelemetry-sdk/src/logs/log_processor.rs +++ b/opentelemetry-sdk/src/logs/log_processor.rs @@ -524,12 +524,7 @@ where return LogResult::Ok(()); } - let log_vec: Vec<(&LogRecord, &InstrumentationScope)> = batch - .iter() - .map(|log_data| (&log_data.0, &log_data.1)) - .collect(); - - let export = exporter.export(LogBatch::new(log_vec.as_slice())); + let export = exporter.export(LogBatch::new_with_owned_data(batch)); let export_result = futures_executor::block_on(export); // Clear the batch vec after exporting From 625c2ad8c7b3ec6139cce3b5d92df1fed99466ff Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Mon, 30 Dec 2024 22:24:31 +0000 Subject: [PATCH 2/6] Code changes --- opentelemetry-sdk/src/export/logs/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/opentelemetry-sdk/src/export/logs/mod.rs b/opentelemetry-sdk/src/export/logs/mod.rs index ac29e1ad16..b5fe3ffeb5 100644 --- a/opentelemetry-sdk/src/export/logs/mod.rs +++ b/opentelemetry-sdk/src/export/logs/mod.rs @@ -23,12 +23,12 @@ pub struct LogBatch<'a> { /// The `LogBatchData` enum represents the data field of a `LogBatch`. /// It can either be: -/// - A mutable reference to a vector of tuples, where each tuple consists of a `LogRecord` and an `InstrumentationScope`. +/// - A shared reference to a vector of tuples, where each tuple consists of a `LogRecord` and an `InstrumentationScope`. /// - Or it can be a slice of tuples, where each tuple consists of a reference to a `LogRecord` and a reference to an `InstrumentationScope`. #[derive(Debug)] #[allow(clippy::vec_box)] // TODO: Revisit this. Clippy complains about using Box in a Vec, but it's done here for performant moves of the data between channel and the vec. enum LogBatchData<'a> { - BorrowedVec(&'a mut Vec>), // Used by BatchProcessor which clones the LogRecords for its own use. + BorrowedVec(&'a Vec>), // Used by BatchProcessor which clones the LogRecords for its own use. BorrowedSlice(&'a [(&'a LogRecord, &'a InstrumentationScope)]), } @@ -53,9 +53,9 @@ impl<'a> LogBatch<'a> { } } - #[allow(clippy::vec_box)] // TODO: Revisit this. Clippy complains about using Box in a Vec, but it's done here for performant moves of the data between channel and the vec. + #[allow(clippy::vec_box)] // Clippy complains about using Box in a Vec, but it's done here for performant moves of the data between channel and the vec. pub(crate) fn new_with_owned_data( - data: &'a mut Vec>, + data: &'a Vec>, ) -> LogBatch<'a> { LogBatch { data: LogBatchData::BorrowedVec(data), From bae9d950fc72a0c376d36d9c32ad0fc2f81931c5 Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Mon, 30 Dec 2024 22:25:26 +0000 Subject: [PATCH 3/6] Code changes --- opentelemetry-sdk/src/export/logs/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/export/logs/mod.rs b/opentelemetry-sdk/src/export/logs/mod.rs index b5fe3ffeb5..7cd84a9ca2 100644 --- a/opentelemetry-sdk/src/export/logs/mod.rs +++ b/opentelemetry-sdk/src/export/logs/mod.rs @@ -26,7 +26,7 @@ pub struct LogBatch<'a> { /// - A shared reference to a vector of tuples, where each tuple consists of a `LogRecord` and an `InstrumentationScope`. /// - Or it can be a slice of tuples, where each tuple consists of a reference to a `LogRecord` and a reference to an `InstrumentationScope`. #[derive(Debug)] -#[allow(clippy::vec_box)] // TODO: Revisit this. Clippy complains about using Box in a Vec, but it's done here for performant moves of the data between channel and the vec. +#[allow(clippy::vec_box)] // Clippy complains about using Box in a Vec, but it's done here for performant moves of the data between channel and the vec. enum LogBatchData<'a> { BorrowedVec(&'a Vec>), // Used by BatchProcessor which clones the LogRecords for its own use. BorrowedSlice(&'a [(&'a LogRecord, &'a InstrumentationScope)]), From e2071b50837d00d2ac1b316983925d7c91ed6197 Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Tue, 31 Dec 2024 06:36:56 +0000 Subject: [PATCH 4/6] Simplify code --- opentelemetry-sdk/src/export/logs/mod.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/opentelemetry-sdk/src/export/logs/mod.rs b/opentelemetry-sdk/src/export/logs/mod.rs index 7cd84a9ca2..4fc7845b41 100644 --- a/opentelemetry-sdk/src/export/logs/mod.rs +++ b/opentelemetry-sdk/src/export/logs/mod.rs @@ -23,12 +23,11 @@ pub struct LogBatch<'a> { /// The `LogBatchData` enum represents the data field of a `LogBatch`. /// It can either be: -/// - A shared reference to a vector of tuples, where each tuple consists of a `LogRecord` and an `InstrumentationScope`. -/// - Or it can be a slice of tuples, where each tuple consists of a reference to a `LogRecord` and a reference to an `InstrumentationScope`. +/// - A shared reference to a slice of tuples, where each tuple consists of an owned `LogRecord` and an owned `InstrumentationScope`. +/// - Or it can be a shared reference to a slice of tuples, where each tuple consists of a reference to a `LogRecord` and a reference to an `InstrumentationScope`. #[derive(Debug)] -#[allow(clippy::vec_box)] // Clippy complains about using Box in a Vec, but it's done here for performant moves of the data between channel and the vec. enum LogBatchData<'a> { - BorrowedVec(&'a Vec>), // Used by BatchProcessor which clones the LogRecords for its own use. + BorrowedVec(&'a [Box<(LogRecord, InstrumentationScope)>]), // Used by BatchProcessor which clones the LogRecords for its own use. BorrowedSlice(&'a [(&'a LogRecord, &'a InstrumentationScope)]), } @@ -53,9 +52,8 @@ impl<'a> LogBatch<'a> { } } - #[allow(clippy::vec_box)] // Clippy complains about using Box in a Vec, but it's done here for performant moves of the data between channel and the vec. pub(crate) fn new_with_owned_data( - data: &'a Vec>, + data: &'a [Box<(LogRecord, InstrumentationScope)>], ) -> LogBatch<'a> { LogBatch { data: LogBatchData::BorrowedVec(data), From fc37341a4f40e3a0253f96de24780968ee32796b Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Tue, 31 Dec 2024 06:38:12 +0000 Subject: [PATCH 5/6] Fix code comment --- opentelemetry-sdk/src/export/logs/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/export/logs/mod.rs b/opentelemetry-sdk/src/export/logs/mod.rs index 4fc7845b41..4602b2f52b 100644 --- a/opentelemetry-sdk/src/export/logs/mod.rs +++ b/opentelemetry-sdk/src/export/logs/mod.rs @@ -23,7 +23,7 @@ pub struct LogBatch<'a> { /// The `LogBatchData` enum represents the data field of a `LogBatch`. /// It can either be: -/// - A shared reference to a slice of tuples, where each tuple consists of an owned `LogRecord` and an owned `InstrumentationScope`. +/// - A shared reference to a slice of boxed tuples, where each tuple consists of an owned `LogRecord` and an owned `InstrumentationScope`. /// - Or it can be a shared reference to a slice of tuples, where each tuple consists of a reference to a `LogRecord` and a reference to an `InstrumentationScope`. #[derive(Debug)] enum LogBatchData<'a> { From 94c854abc44d83bc9afc10630f81b543e1af46be Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Tue, 31 Dec 2024 06:40:31 +0000 Subject: [PATCH 6/6] Code changes --- opentelemetry-sdk/src/logs/log_processor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/logs/log_processor.rs b/opentelemetry-sdk/src/logs/log_processor.rs index b5fccd51f8..b6284b87c6 100644 --- a/opentelemetry-sdk/src/logs/log_processor.rs +++ b/opentelemetry-sdk/src/logs/log_processor.rs @@ -524,7 +524,7 @@ where return LogResult::Ok(()); } - let export = exporter.export(LogBatch::new_with_owned_data(batch)); + let export = exporter.export(LogBatch::new_with_owned_data(batch.as_slice())); let export_result = futures_executor::block_on(export); // Clear the batch vec after exporting