opentelemetry-cpp icon indicating copy to clipboard operation
opentelemetry-cpp copied to clipboard

[Trace API] Tracer::StartSpan() missing api for a single link.

Open marcalff opened this issue 3 years ago • 5 comments
trafficstars

In the spec for Span creation, for specifying links:

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#specifying-links

The Span creation API MUST provide:

An API to record a single Link where the Link properties are passed as arguments. (...).

Every StartSpan method on Tracer, when taking links as input, expects many links (iterators on collections, initializer lists, etc) at once.

There are no apis that can be used to add a single link, with optional attributes, when creating a span.

tags: trace api, spec compliance.

marcalff avatar Jun 27 '22 09:06 marcalff

Thanks for the issue. Yes, the API is not meeting the specs here. The problem is - all the links can be added only while creating a span and not afterward. So it was easy to support iterable over links. Otherwise, we should have added an overloaded StartSpan() method to support both single and multiple links, and that would have complicated the API.

Had a quick look at python and JS implementations, both also support only a list of links for the StartSpan method.

lalitb avatar Jun 27 '22 20:06 lalitb

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

github-actions[bot] avatar Aug 27 '22 02:08 github-actions[bot]

Method Tracer::StartSpan() takes the following input:

  • a span name (1 combination)
  • attributes (K1 combinations)
  • links (K2 combinations)
  • options (K3 combination)

so when exposing all variations in several SpartSpan() methods, this leads to K1 * K2 * K3 methods.

This is not desirable, and not maintainable, agreed.

What is missing is a helper for links, that can be passed as parameter to StartSpan.

For example:

class SpanLinks {

// To construct links directly:

  SpanLinks(); // empty links
  SpanLinks(const SpanContext& span_context); // single link, no attributes
  SpanLinks(const SpanContext& span_context, xxx attributes); // single link, with attributes
  SpanLinks (xxx links); // multiple links

// To add more links:

  void add(const SpanContext& span_context);
  void add(const SpanContext& span_context, xxx attributes);
}

class SpanLinks will have to support the SpanContextKeyValueIterable interface.

Then, the application instrumentation can do something like:

SpanLinks no_links();
tracer->StartSpan("name", attributes, no_links, options);

SpanLinks single_link(linked_span_context);
tracer->StartSpan("name", attributes, single_link, options);

SpanLinks many_link();
many_links.add(...);
many_links.add(...);
many_links.add(...);
tracer->StartSpan("name", attributes, many_link, options);

etc

With this, the complexity becomes:

  • K1 combinations to build attributes, with a helper
  • K2 combinations to build links, with a helper
  • K3 combinations (2, to provide a StartSpanOptions struct or not, really), for options

and the total complexity of the API is K1 + K2 + K3.

marcalff avatar Aug 31 '22 12:08 marcalff

The idea behind the original implementation was to avoid any dedicated types/objects for Links and Events. Currently, there is no intermediate span-data storage at the API/SDK level. Please correct if wrong, it looks like SpanLinks will now maintain span_context and attributes internally before they are added to Span?

lalitb avatar Sep 01 '22 05:09 lalitb

As discussed in SIG meeting, a possible mitigation is to add an example in the documentation, to show in practice how to add a link to a span, given a trace context.

marcalff avatar Sep 14 '22 20:09 marcalff