How could i return dynamic attributes in a sampler

[xiehuc]

Before opening a feature request against this repo, consider whether the feature should/could be implemented in the other OpenTelemetry client libraries. If so, please open an issue on opentelemetry-specification first.

Is your feature request related to a problem?

::opentelemetry::sdk::trace::SamplingResult MySampler::ShouldSample(
    const ::opentelemetry::trace::SpanContext& parent_context,
    ::opentelemetry::trace::TraceId trace_id, ::opentelemetry::nostd::string_view name,
    ::opentelemetry::trace::SpanKind span_kind,
    const ::opentelemetry::common::KeyValueIterable& attributes,
    const ::opentelemetry::trace::SpanContextKeyValueIterable& links) noexcept 

    result.attributes =
        std::make_unique<const std::map<std::string, opentelemetry::common::AttributeValue>>(
            std::move(kv));

SampleingResult has attributes, but it's value is AttributeValue, just reference, not own memory.

so, my question is, where could i attach original value string to this session, so it could free'd after span send.

parent_context?
trace_state?

no way !!! this API just could not make it work but still keeps it exist.

i think about thread_local, object pool, delay timer, std::string allocator, but all of this just make a simple things becomes very complex。

Describe the solution you'd like
no idea, dead lock.

Describe alternatives you've considered

Additional context

[lalitb]

Valid concern. The SamplingResult::attributes needs to stay valid for the duration of the Tracer::StartSpan() method's execution (not till the span's end). And it is difficult for Sampler implementation to guarantee this as attributes value is AttributeValue which doesn't own memory for non-primitive types. One approach could be to change SamplingResult::attributes to use OwnedAttributeValue instead of AttributeValue, which will give the ownership of attributes to SamplingResult.

struct SamplingResult
{
   ...
   std::unique_ptr<const std::map<std::string, opentelemetry::sdk::common::OwnedAttributeValue>> attributes;
   ...
}

Span::SetAttribute() takes AttributeValue as an argument, so we need conversions here. Need more discussion/thoughts to see if there is a better solution.

[xiehuc]

i rethink the code, found that:

class Span {
  void SetAttribute(nostd::string_view key,
                    const opentelemetry::common::AttributeValue &value) noexcept override;
}

the AttributeValue is string_view, is totally OK.

because Span's underline is sdk/src/trace/span.cc

would call recordable_->SetAttribute(key, value) would call

  auto *attribute = span_.add_attributes();
  OtlpPopulateAttributeUtils::PopulateAttribute(attribute, key, value);

span_ is protobuf, would finally hold value. so string_string is just a bridge.

that is

std::string value("value");
span.SetAttribute("key", value); 

just ok, SetAttribute finally make protobuf hold value.

but in SamplingResult things are totally different.

when a Sampler set attribute

::opentelemetry::sdk::trace::SamplingResult GalileoSampler::ShouldSample(......) noexcept {
  std::string value("value");
  std::map<std::string, opentelemetry::common::AttributeValue> kv_map{{"key", value}};
  result.attributes = std::make_unique(kv_map);
  return result;
}

after ShouldSample return, value would immediately free，so attribute hold a bad value.

on parent function,

even if it call SetAttribute after, the value is just a bad value.

[xiehuc]

the solution is simple.

let ShouldSample can call SetAttribute .

1. put span in ShouldSample Argument
2. or put a callback in ShouldSample Argument
3. delete SampleResult attributes. it really misleading people.
4. SampleResult hold value, do not use opentelemetry::common::AttributeValue type

[lalitb]

the solution is simple.

let ShouldSample can call SetAttribute .

Well not that easy :) The Span doesn't exist when ShouldSample is called. As Span is created based on sampling decision.

put span in ShouldSample Argument
or put a callback in ShouldSample Argument

As mentioned above, this needs Span to already exist when ShouldSample is called.

delete SampleResult attributes. it really misleading people.

We could potentially implement this, but it would result in our implementation becoming non-compliant with the specification.

SampleResult hold value, do not use opentelemetry::common::AttributeValue type

Yes, this is one of the solutions, to instead use sdk::common::OwnedAttributeValue. The SDK in principle doesn't do any intermediate storage on span objects and instead serializes eagerly as methods are called. This change will go against this guiding principle.

[github-actions]

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