Adding resource to TracerProvider

[ThomsonTan]

OpenTelemetry Resource SDK spec mentioned associating resource to TraceProvider, which seems missing the current TracerProvider implementation. Actually it seems resource is not implemented yet.

When used with distributed tracing, a resource can be associated with the TracerProvider when the TracerProvider is created

[jsuereth]

A summary of Resource (and InstrumentationLibrary) design so far.

Tl;DR: Current proposal is to:

• Update Tracer so that it has an instance of InstrumentationLibrary (a name/version set of strings)
• Update TracerProvider to abide by spec regarding creation of Tracers.
• Update SpanProcessor and Exporter in trace SDK to leverage ExportableSpan (or some new type that holds both the Recordable and a reference to Tracer. This may have larger implications design/ownership in BatchSpanProcessor. The class is ALMOST:

  struct ExportableSpan {
       std::unique_ptr<Recordable> recordable;
       std::shared_ptr<Tracer> tracer;
  }
  

• Update OTLP exporter.
  • ideally we assume ONE resource for all spans seen due to new ownership rules. If not we need to group spans by resource.
  • We can sort by Tracer in some way to be able to group spans by InstrumentationLibrary as per the protocol.
  • We can pull both Resource and InstrumentationLibrary from the newly attached Tracer on ExportableSpan (or whatever we call it).

A very messy solution can be seen in this PR: #590 (previous version: #580)

For context/comparison the previous proposal from @lalitb:

• We add methods to Recordable which allow specifying resource (and InstrumentationLibrary, i.e. the name/version specified when creating a Tracer)
• When creating a span, immediately set the known Resource and InstrumentationLibrary attributes on Recordable
• non-OTLP implementations of Recordable implement these new methods as no-ops.
• OTLP exporter needs to store resource + instrumentation library and perform "rebundling" on spans later.

This is under the assumption that the new proposal leads to less copying of data which would be the primary performance concern. I'm not 100% confident having seen the prototype implementation, but I think we should be able to work out the kinks and it gives us a lot of flexibility going forward.

[apmcasai]

Hi,

Wondering if there's any priority to get this fixed? I'm using the library to record to elasticsearch but currently my service get recorded as "unknown" which is terrible for readability and might also collide if we add more CPP services...

[lalitb]

@apmcasai - Yes this is on priority for us, and we are working to add it sooner.