[exporterhelper] Add span links across batcher when merging multiple requests

[jade-guiton-dd]

Description

Continuation of #12318:

• Small change to avoid adding both a parent span relationship and a span link in cases where no merge is made
• Fix failing tests by removing them: Those tests relate to cancelling a batch of export requests if the context for one of them is cancelled. I'm not sure how useful this logic is, or if it makes sense to cancel unrelated requests that happened to be batched with the one that was cancelled. If there are objections to this, I can try to reimplement this logic.

Link to tracking issue

Updates #12212 (remaining: persist parent span across persistent queue)

(edit by @mx-psi):

• Fixes [exporter/otlp] Review telemetry #11140
• Fixes [exporter/otlphttp] Review telemetry #11141

[codecov]

Codecov Report

Attention: Patch coverage is 90.62500% with 3 lines in your changes missing coverage. Please review.

Project coverage is 91.42%. Comparing base (564818f) to head (e3dd283).
Report is 13 commits behind head on main.

Files with missing lines	Patch %	Lines
...xporterhelper/internal/queuebatch/batch_context.go	84.21%	2 Missing and 1 partial ⚠️

❌ Your patch status has failed because the patch coverage (90.62%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #12768      +/-   ##
==========================================
- Coverage   91.43%   91.42%   -0.01%     
==========================================
  Files         487      488       +1     
  Lines       26808    26839      +31     
==========================================
+ Hits        24511    24537      +26     
- Misses       1814     1818       +4     
- Partials      483      484       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jmacd]

I agree that this test should be removed. It does not make sense, IMO, to cancel a whole batch because one of the inputs timed out, for example.

[jmacd]

I agree with removing this test. I assume other tests cover this behavior, it's not a very strong test condition.

[mx-psi]

@bogdandrutu @dmitryax I will merge this on Friday unless you block/comment by then

[jade-guiton-dd]

@jmacd We will probably merge this as is, but I created a separate issue for your suggestion from the Collector stability meeting about how to combine the incoming timeouts: #12809. I may not have time to work on it myself however.

[jackgopack4]

this is a tidy solution, just left one note about whether we should consider adding a trivial test case to get rid of the warning

[jackgopack4]

I see the unit test nag and I am wondering if we have a mental model for when context might be nil. If it is unlikely to happen I'm ok with adding a simple test or test case to exercise that code and calling it good.

[jade-guiton-dd]

I think passing in nil Contexts is considered very bad practice, and almost no code in collector/collector-contrib checks for that, so I think any receivers that do that would end up crashing the Collector very early anyway.

So I don't think that check is really needed (it's a holdover from the previous PR); if we really want the test coverage for its own sake, I'd say we remove the check entirely instead.