Keep supporting InstrumentationLibrarySpans until the attribute is removed

[dmathieu]

#2748 removed support for InstrumentationLibrarySpans and renamed it to ScopeSpan, because that attribute has been renamed in protobuf.
However, fully removing that support in a single change is breaking apps here and there, as it means folks have to upgrade both the library and their collector at the same time.

This change brings back support for InstrumentationLibrarySpans, so we send both fields and can still support collectors that rely on the deprecated field.
Once proto fully removes that field, we can remove support for it here (which may require a major release?).

• Related to no more spans sent with 1.6.1 #2768
• See https://cloud-native.slack.com/archives/C01NPAXACKT/p1649169259491669
• See https://cloud-native.slack.com/archives/C01NPAXACKT/p1649154189451329

[codecov]

Codecov Report

Merging #2769 (86afeca) into main (c91da41) will decrease coverage by 0.0%.
The diff coverage is 54.3%.

@@           Coverage Diff           @@
##            main   #2769     +/-   ##
=======================================
- Coverage   76.8%   76.7%   -0.1%     
=======================================
  Files        181     181             
  Lines      12189   12231     +42     
=======================================
+ Hits        9363    9385     +22     
- Misses      2601    2621     +20     
  Partials     225     225             

Impacted Files	Coverage Δ
.../otlp/otlptrace/internal/otlptracetest/otlptest.go	0.0% <0.0%> (ø)
...lptrace/internal/tracetransform/instrumentation.go	100.0% <100.0%> (ø)
...ers/otlp/otlptrace/internal/tracetransform/span.go	96.8% <100.0%> (+0.3%)	⬆️
sdk/trace/batch_span_processor.go	80.2% <0.0%> (-1.0%)	⬇️
exporters/jaeger/jaeger.go	91.1% <0.0%> (+0.8%)	⬆️

[Aneurysm9]

I'm not sure this is the correct conclusion to draw from the available evidence. #2748 was not included in the v1.6.1 release and thus could not have changed the data sent over the wire. Further, the Scope fields are in the OTLP proto at the same position as the Library fields were, meaning that #2748 should also result in no change to the data sent over the wire as it changed all uses of the Library fields. Putting data in both locations would be duplicative and a significant performance (and potentially cost) penalty.

[dmathieu]

I've removed the Fixes for #2768.
My thinking is that it will probably take months for providers (especially the proprietary ones) to update their proto to use the new attribute. During that time, folks will therefore likely not be able to upgrade their libraries.

Technically, dropping support for the old field right away also is a breaking change. So it would require a major release.

[Aneurysm9]

go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.6.1 appears to work fine and I am able to use it to send traces to an ADOT collector v0.17.0 instance (based on upstream v0.45.0), and to a collector-contrib v0.48.0 instance, and from there to X-Ray. This is as expected.

When I force the use of go.opentelemetry.io/proto/otlp v0.15.0 with a replace directive then spans are no longer received at X-Ray from the ADOT v0.17.0 collector instance. This is as expected.

otlptracegrpc v1.6.1 uses the InstrumentationLibrary fields in the protobuf structs. With proto/otlp v0.15.0 those named fields are now in a different position in the message structure and the collector (prior to v0.48.0) does not know what to do with them and thus ignores them and all of the telemetry they contain.

What is not expected is that otlptracegrpc v1.6.1 and proto/otlp v0.15.0 do not result in traces at X-Ray when sent to the collector-contrib v0.48.0 instance. That instance should be remapping the incoming messages before emitting them to the rest of the pipeline.

Using otlptracegrpc main and proto/otlp v0.15.0, which should result in emitting the same data on the wire as v1.6.1 and v0.12.1, results in traces making their way to X-Ray from both the ADOT v0.17.0 collector instance and the collector-contrib v0.48.0 instance.

This indicates a potential problem with the conversion logic in the collector and not otlptracegrpc v1.6.1.

I will continue investigating.

[MrAlias]

I think this can be closed with the release of v1.6.2. We now have a solution for users that is to upgrade to the latest version.

[tigrannajaryan]

Unless I am missing something it is expected that this would break. Recompiling 1.6.1 with 0.15.0 will result in all emitted data to go to Protobuf field 1000 which existing recipients such as Collector 0.47.0 do not know about.

This scenario was anticipated (I may have not done a good enough job articulating it) and the way to avoid it was to precisely not do the recompiling without updating the code to use the new field names.

If only the combination of 1.6.1 with 0.15.0 was broken and now 1.6.2 works correctly with all Collector versions I think there is nothing else to be done.

What I do not quite understand is why the end users were affected by this? Replacing OTLP 0.12.1 by 0.15.0 should have resulted in a new Go SDK release with a new version number, right? Why was it still called Go SDK 1.6.1?

Nevermind, this is not the scenario that is broken. 🤦

[tigrannajaryan]

I will continue investigating.

@Aneurysm9 please post an update when you find anything. Thanks for looking.

[tigrannajaryan]

open-telemetry/opentelemetry-collector#5189 should fix this.