Open t-b opened 5 months ago
This looks like a lifetime issue with regarding common::AttributeValue.
You are correct. The ostream-exporter uses ReadWriteLogRecord as recordable implementation, and the attributes are stored as:
std::unordered_map<std::string, opentelemetry::common::AttributeValue> attributes_map_;
where AttributeValue is a (no)std::variant with non-owning values:
using AttributeValue =
nostd::variant<bool,
int32_t,
int64_t,
uint32_t,
double,
const char *,
nostd::string_view,
nostd::span<const bool>,
nostd::span<const int32_t>,
nostd::span<const int64_t>,
nostd::span<const uint32_t>,
nostd::span<const double>,
nostd::span<const nostd::string_view>,
// Not currently supported by the specification, but reserved for future use.
// Added to provide support for all primitive C++ types.
uint64_t,
// Not currently supported by the specification, but reserved for future use.
// Added to provide support for all primitive C++ types.
nostd::span<const uint64_t>,
// Not currently supported by the specification, but reserved for future use.
// See https://github.com/open-telemetry/opentelemetry-specification/issues/780
nostd::span<const uint8_t>>;
So, while using ostream-exporter, you need to ensure that the attribute value remains valid till the log is exported. I understand this is difficult to ensure with batch-processor as the export is async, but since ostream-exporter is only for testing scenario, I think we should be good with this expectation.
I would also be interested to know if the bug is limited to the ostream exporter or also applies to other exporters, we want tot use otlp with grpc and http in production.
The issue is specific to ostream-exporter with batch processor. The OTLP log exporter would directly serialize the attributes in the proto format, and so the lifetime issue won't come into picture.
@lalitb Thanks for the quick reply and explanation. Glad this is only present with the ostream exporter.
But even for testing-only scenarios with the ostream exporter I would prefer a compiletime/runtime error.
Agree, the fix should be straighforward to avoid the crash by using the owning types for attributes ( as done within span-data) https://github.com/open-telemetry/opentelemetry-cpp/blob/0ca7d707a505ca482b07e1ede1b19c25e198380d/sdk/include/opentelemetry/sdk/trace/span_data.h#L314
I will pick up the issue after couple of weeks if it is not fixed.
@t-b Also, apart from current issue I described, if your code has something like:
auto attributes = opentelemetry::common::MakeAttributes(
{{ "key1" , some_value_1},
{ "key2", some_value_2}};
logger->Debug("Logging attributes", attributes);
you need to ensure that the some_value_1
and some_value_2
are not released before the call to logger->Debug(). This is valid for all the exporters.
Accepted - to fix ostream exporter to behave the same as OTLP
This issue was marked as stale due to lack of activity.
Unstale.
This issue was marked as stale due to lack of activity.
Describe your environment OS: Debian bookworm GCC: 12.2.0 Version: 054b0dc2 ([RELEASE] Release opentelemetry-cpp version 1.15.0 (#2639), 2024-04-21), also present in 1.11.0.
Steps to reproduce
Outputs:
If I use the
SimpleLogRecordProcessor
there is no garbled output. And if I pass in aconst char*
viasomeString.c_str()
and use the batched processor I also get garbled output.What is the expected behavior? That the strings I sent are outputted properly.
What is the actual behavior? Garbled output.
Additional context Add any other context about the problem here.
I would also be interested to know if the bug is limited to the ostream exporter or also applies to other exporters, we want tot use otlp with grpc and http in production.
This looks like a lifetime issue with regarding common::AttributeValue.