Attributes from Samplers not being added to sampled Spans

[paulosman]

The sampling section of the Tracing SDK specification specifies that the return value of the ShouldSample method contains "A set of span Attributes that will also be added to the Span.".

When using a custom sampler with the Go SDK, attributes returned from the Sampler are only set on the root Span in a Trace, not on each of the individual spans. I tested with the Python SDK, and in that implementation, Attributes are being set on every span.

The use case we need this for is to be able to set the Sample Rate for each span sent to Honeycomb. Our backend uses the sample rate when calculating aggregate functions on events. Here's the sampler implementation we're using for a proof of concept.

Using this sample app, we get the following output:

[
  {
    "SpanContext": {
    ...
    },
    "Attributes": null,
  }
]

On each of the Spans, except to the root span:

[
  {
     "SpanContext": {
      ...
  },
  "Attributes": [
    {
      "Key": "SampleRate",
      "Value": {
        "Type": "INT32",
        "Value": 2
      }
    },
    ...
    ]
  }
]

Using the Python sample app, I'm able to see the Attribute on every span:

    ...
    "attributes": {
        "SampleRate": 2
    },
    ...

[paulosman]

btw, I'm happy to take a crack at a PR for this but want to make sure others agree with the framing of the problem first.

[Aneurysm9]

I can't access the sample application, perhaps the repo is private?

It looks like we might be slightly overzealous in attempting to avoid invoking samplers. This may be a holdover from before the ParentBased sampler was made significantly more flexible. Can you confirm that the issue is in fact that your sampler is not being invoked at all for child spans rather than the attributes not being set?

[paulosman]

My apologies, the repo is public now in case it's still helpful.

I've confirmed - the ShouldSample method is only being called once per trace, and not once per span. I tried commenting out that guard that you linked to and indeed that results in the expected behavior - the sampler is consulted for each span and attributes are set properly.

[Aneurysm9]

Excellent! @paulosman do you want me to assign this issue to you? I'll add it to the agenda for the Go SIG meeting this afternoon to see if anyone has any reason why we shouldn't just strip that guard out.

[paulosman]

@Aneurysm9 Yeah sure, that sounds good! I'll join the SIG meeting today.