Fix compile with clang 16 and libc++

[owent]

Fixes #2241

Changes

• Move destructor of SimpleLogRecordProcessor into .cc .
• Use std::vector::assign to replace std::fill .

For significant contributions please make sure you have completed the following items:

• CHANGELOG.md updated for non-trivial changes
• Unit tests have been added
• Changes in public API reviewed

[marcalff]

About the first issue:

I think the root cause is the forward declaration of LogRecordExporter.

Because of std::unique_ptr<LogRecordExporter>, the compiler needs to see the LogRecordExporter destructor, and the header file that declares it.

Please try to include the header file for LogRecordExporter instead.

[marcalff]

Per build logs in this PR:

2023-07-20T14:35:30.4501309Z Warning: include-what-you-use reported diagnostics:
2023-07-20T14:35:30.4501835Z 
2023-07-20T14:35:30.4502487Z /home/runner/work/opentelemetry-cpp/opentelemetry-cpp/sdk/include/opentelemetry/sdk/logs/simple_log_record_processor.h should add these lines:
2023-07-20T14:35:30.4503155Z #include <bits/chrono.h>                           // for microseconds
2023-07-20T14:35:30.4503659Z #include "opentelemetry/sdk/logs/exporter.h"       // for LogRecordExporter
2023-07-20T14:35:30.4504200Z #include "opentelemetry/sdk/logs/recordable.h"     // for Recordable
2023-07-20T14:35:30.4504529Z 
2023-07-20T14:35:30.4505149Z /home/runner/work/opentelemetry-cpp/opentelemetry-cpp/sdk/include/opentelemetry/sdk/logs/simple_log_record_processor.h should remove these lines:
2023-07-20T14:35:30.4505824Z - #include <chrono>  // lines 8-8
2023-07-20T14:35:30.4506531Z - namespace opentelemetry { namespace v1 { namespace sdk { namespace logs { class LogRecordExporter; } } } }  // lines 20-20
2023-07-20T14:35:30.4507384Z - namespace opentelemetry { namespace v1 { namespace sdk { namespace logs { class Recordable; } } } }  // lines 21-21

Please add the following two lines:

#include "opentelemetry/sdk/logs/exporter.h"
#include "opentelemetry/sdk/logs/recordable.h"

and remove the matching forward declarations,
in file sdk/include/opentelemetry/sdk/logs/simple_log_record_processor.h

Looks like unique_ptr is preventing to use forward declarations.

[owent]

About the first issue:

I think the root cause is the forward declaration of LogRecordExporter.

Because of std::unique_ptr<LogRecordExporter>, the compiler needs to see the LogRecordExporter destructor, and the header file that declares it.

Please try to include the header file for LogRecordExporter instead.

The problem is ~SimpleLogRecordProcessor() override = default; need the definition of LogRecordExporter, because it's a member of SimpleLogRecordProcessor.

BatchLogRecordProcessor, BatchSpanProcessor have the similar declarations, do you think it's better to keep the similar includes here?

[codecov]

Codecov Report

Merging #2242 (37f3dac) into main (a15a9b8) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2242   +/-   ##
=======================================
  Coverage   87.34%   87.34%           
=======================================
  Files         169      169           
  Lines        4902     4902           
=======================================
  Hits         4281     4281           
  Misses        621      621           

Impacted Files	Coverage Δ
sdk/src/metrics/data/circular_buffer.cc	100.00% <100.00%> (ø)

[marcalff]

About the first issue:
I think the root cause is the forward declaration of LogRecordExporter.
Because of std::unique_ptr<LogRecordExporter>, the compiler needs to see the LogRecordExporter destructor, and the header file that declares it.
Please try to include the header file for LogRecordExporter instead.

The problem is ~SimpleLogRecordProcessor() override = default; need the definition of LogRecordExporter, because it's a member of SimpleLogRecordProcessor.

BatchLogRecordProcessor, BatchSpanProcessor have the similar declarations, do you think it's better to keep the similar includes here?

Independently of forwards, I agree that moving code from *.h to *.cc is a good change in general.

My concern is that, even when this fix the immediate issue, there might be more cases where the use of forward declarations with std::unique_ptr causes problems (this is specific to unique_ptr, shared_ptr is working properly and does not need to see the destructor of T).

Revising every forward used in every std::unique_ptr will be a separate change anyway, to analyze in more details, so approving this change.

[marcalff]

LGTM