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

(batchprocessor): Add tracing support

Open jmacd opened this issue 1 year ago • 4 comments

Description

The batch processor is frequently responsible for breaking trace context for data passing through the collector. This fixes the batch processor to either continue the trace context (if one parent) or to link the parent contexts. The motivation for this change goes beyond tracing. The same data structure is used in following PRs to address #11308, in which the pendingItem data structure will gain a channel so the export can inform the waiter about completion(s), in which the parentCtx.Done() channel is used to avoid blocking when the caller has already returned.

Link to tracking issue

Part of #11308

Testing

Two new tests for single-parent and multi-parent trace data.

Documentation

Added.

jmacd avatar Oct 01 '24 15:10 jmacd

Codecov Report

Attention: Patch coverage is 99.08257% with 1 line in your changes missing coverage. Please review.

Project coverage is 91.51%. Comparing base (3456b43) to head (e52747a). Report is 56 commits behind head on main.

Files with missing lines Patch % Lines
processor/batchprocessor/batch_processor.go 99.08% 0 Missing and 1 partial :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11324      +/-   ##
==========================================
- Coverage   92.14%   91.51%   -0.63%     
==========================================
  Files         433      433              
  Lines       20396    23701    +3305     
==========================================
+ Hits        18794    21691    +2897     
- Misses       1238     1642     +404     
- Partials      364      368       +4     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Oct 01 '24 15:10 codecov[bot]

LGTM. @bogdandrutu, you had a lot of feedback on this PR.

Having a lot of feedback is it bad or good? Or why did you mentioned this?

Have your changes been addressed?

Not yet, some of the answers to my comments I am unable to understand (most likely because of my IQ level, or other factors). I am still in the understanding mode, and I cannot approve something I don't understand the complexity.

bogdandrutu avatar Oct 08 '24 03:10 bogdandrutu

@evan-bradley What do you think? This is a simple PR to add tracing to a critical component, and I have tried to explain that this is only one of the steps involved in fixing #11308.

jmacd avatar Oct 08 '24 19:10 jmacd

Not yet, some of the answers to my comments I am unable to understand

@bogdandrutu please clarify. I can't guess at which parts of this PR and the overall #11308 are causing trouble.

jmacd avatar Oct 09 '24 18:10 jmacd

Disappointed.

jmacd avatar Oct 30 '24 21:10 jmacd