Make a BatchProcessor class which both BatchSpanRecordProcessor and BatchLogRecordProcessor can use #4562
Conversation
…-python into refactor_blrp
opentelemetry-sdk/tests/shared_internal/test_batch_processor.py
Outdated
Show resolved
Hide resolved
|
Nevermind.. was able to get the test working |
opentelemetry-sdk/src/opentelemetry/sdk/_shared_internal/__init__.py
Outdated
Show resolved
Hide resolved
opentelemetry-sdk/src/opentelemetry/sdk/_shared_internal/__init__.py
Outdated
Show resolved
Hide resolved
| token = attach(set_value(_SUPPRESS_INSTRUMENTATION_KEY, True)) | ||
| try: | ||
| self._exporter.export( | ||
| [ # pyright: ignore [reportArgumentType] |
There was a problem hiding this comment.
Ack switched to generics.. I still get a pyright issue thrown here, because now it can't figure out that the generic _exporter has an export method.. But I think it's cleaner then before anyway
|
|
||
|
|
||
| class BatchLogRecordProcessor(LogRecordProcessor): | ||
| class BatchLogRecordProcessor(BatchProcessor, LogRecordProcessor): |
There was a problem hiding this comment.
I'm not a huge fan of multiple inheritance. Wdyt of using composition instead of inheritance here?
There was a problem hiding this comment.
Tried doing that, but then the emit and shutdown methods need to be accessed like batch_record_processor._batch_processor.emit() which breaks stuff, also the linter complains about the BatchLogRecordProcessor being initialized directly because it inherits from an abstract class (LogRecordProcessor) and doesn't implement those methods.
Description
Make a
BatchProcessorclass which bothBatchSpanRecordProcessorandBatchLogRecordProcessorcan use.Move
BatchProcessorinto a new_shared_internalfolder in the SDK, so both processors can make use of it.In a follow up PR, I will update
BatchSpanRecordProcessorto use this new class.Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Added unit tests
Does This PR Require a Contrib Repo Change?
Checklist: