Adds a custom MarshalJSON method for trace.Link

[IrisTuntun]

Adds a custom MarshalJSON method for trace.Link to marshal all fields.
Should resolve #1820

[codecov]

Codecov Report

Merging #1852 (992c273) into main (392a44f) will increase coverage by 0.1%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##            main   #1852     +/-   ##
=======================================
+ Coverage   78.6%   78.8%   +0.1%     
=======================================
  Files        137     137             
  Lines       7298    7307      +9     
=======================================
+ Hits        5737    5758     +21     
+ Misses      1316    1304     -12     
  Partials     245     245             

Impacted Files	Coverage Δ
trace/trace.go	93.7% <100.0%> (+6.9%)	⬆️
exporters/trace/jaeger/jaeger.go	92.8% <0.0%> (+0.5%)	⬆️

[MrAlias]

Should we just not embed the SpanContext in a Link? Instead make it a field?

[IrisTuntun]

Should we just not embed the SpanContext in a Link? Instead make it a field?

Then it will be another issue. I wonder if I should make the change in this PR.

[MrAlias]

Then it will be another issue. I wonder if I should make the change in this PR.

I would get the opinion of another approver/maintainer before restarting.

[Aneurysm9]

Should we just not embed the SpanContext in a Link? Instead make it a field?

That would also work. I don't have any context for why SpanContext was embedded rather than included as a field, but I'd presume that it was to avoid the need to write passthrough methods to get at the linked trace and span IDs. That said, it doesn't look like the spec provides for any sort of interface for interacting with Links and API Spans are write-only, so once they're added there's no way to access their properties through the API.

As I was reviewing #1846 I was wondering whether the Link type should be moved as well. It would be slightly more complicated since the API does have a Span#AddLink() method, but the spec certainly contemplates that taking separate SpanContext and Attributes arguments, so we could restructure.

[Aneurysm9]

I see I didn't look closely enough at the issue #1846 was addressing and this topic is discussed there. Added some thoughts: #1845 (comment).

[MrAlias]

Closing as the decision outlined in #1845 (comment) is to remove the Link type from the API. We can recreate the type in the SDK without embedding the SpanContext.

[MrAlias]

@IrisTuntun we realized that we cannot remove the link and we should instead go with the un-embed solution: #1845 (comment)

Is this something you would like to open a PR for?

[IrisTuntun]

@IrisTuntun we realized that we cannot remove the link and we should instead go with the un-embed solution: #1845 (comment)

Is this something you would like to open a PR for?

Sure, I can take this. Will open a PR for it.