OStreamLogRecordExporter doesn't copy message data and attributes

[lalitb]

OStreamLogExporter uses ReadWriteLogRecord as the recordable implementation, and this implementation doesn't maintain copy of message body and attributes:

https://github.com/open-telemetry/opentelemetry-cpp/blob/46e20a42aef521b9cc1b7207310bfefcd490741a/sdk/include/opentelemetry/sdk/logs/read_write_log_record.h#L190-L191

This can result in the use-after-free crash in case the message body or attributes are cleaned-up before the data is processed by batch processor as in below:

#include "opentelemetry/logs/provider.h"
#include "opentelemetry/sdk/logs/batch_log_record_processor.h"
#include "opentelemetry/sdk/logs/logger_provider.h"

#include "opentelemetry/exporters/ostream/log_record_exporter.h"
#include "opentelemetry/sdk/logs/batch_log_record_processor_factory.h"
#include "opentelemetry/sdk/logs/logger_provider_factory.h"
#include "opentelemetry/sdk/logs/processor.h"

namespace logs_api = opentelemetry::logs;
namespace logs_sdk = opentelemetry::sdk::logs;
namespace logs_exporter = opentelemetry::exporter::logs;

using namespace std;
namespace nostd = opentelemetry::nostd;

int
main(int argc, char** argv)
{
    logs_sdk::BatchLogRecordProcessorOptions batch_opts {};

    auto os_exporter = unique_ptr<logs_sdk::LogRecordExporter>(
        new opentelemetry::exporter::logs::OStreamLogRecordExporter);
    auto os_processor =
        logs_sdk::BatchLogRecordProcessorFactory::Create(move(os_exporter), batch_opts);

    auto provider =
        shared_ptr<logs_api::LoggerProvider>(new logs_sdk::LoggerProvider(move(os_processor)));

    // Set the global logger provider
    logs_api::Provider::SetLoggerProvider(provider);

    auto logger = logs_api::Provider::GetLoggerProvider()->GetLogger("anything");

    string* sp = new string("a message "s + std::to_string(random()) + " is being created "s +
                            std::to_string(random()));
    logger->Info(*sp);
    delete sp;

    sp = new string("a message "s + std::to_string(random()));
    logger->Info(*sp);
    *sp = "Now it is another message entirely";
    delete sp;

    sleep(10);
}

Just like SpanData implementation which is used in OStream span exporter, the ReadWriteLogRecord should also maintain the copy of these data.

[ThomsonTan]

Another thought, should we limit the usage of OStreamLogRecordExporter in async pipeline as the BatchLogRecordProcessor? So we don't need to make copies of the attributes for most common cases.

[lalitb]

The OStreamLogRecordExporter serves as a demonstration model to implement exporters for logs, and it should do it in right way. And also, good to have parity with OStreamSpanExporter which can be used both with Simple and Batch Processor. SpanData in that case maintains the copy of attributes as shared in link above.

[github-actions[bot]]

This issue was marked as stale due to lack of activity.

[ThomsonTan]

Making copy could make sense because the OStream exporter is mostly for debugging purpose.