Skip to content

Sdk builders extended with addProcessorFirst methods#7243

Merged
jack-berg merged 6 commits intoopen-telemetry:mainfrom
onurkybsi:issue-6599
Apr 18, 2025
Merged

Sdk builders extended with addProcessorFirst methods#7243
jack-berg merged 6 commits intoopen-telemetry:mainfrom
onurkybsi:issue-6599

Conversation

@onurkybsi
Copy link
Contributor

Resolves the issue #6599.

@onurkybsi onurkybsi requested a review from a team as a code owner April 2, 2025 20:29
@codecov
Copy link

codecov bot commented Apr 2, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.62%. Comparing base (04677f9) to head (e719247).
Report is 2 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #7243      +/-   ##
============================================
+ Coverage     89.60%   89.62%   +0.01%     
- Complexity     6859     6862       +3     
============================================
  Files           780      780              
  Lines         20728    20735       +7     
  Branches       2018     2018              
============================================
+ Hits          18574    18584      +10     
+ Misses         1514     1512       -2     
+ Partials        640      639       -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.
@onurkybsi onurkybsi changed the title BatchLogRecordProcessor enforced to end of processor pipeline Apr 16, 2025
Copy link
Member

@jack-berg jack-berg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving, but should also add the equivalent method to tracing

* @param processor the log processor
* @return this
*/
public SdkLoggerProviderBuilder addLogRecordProcessorFirst(LogRecordProcessor processor) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name follows the naming convention of Deque#addFirst, albeit "LogRecordProcessor" inserted between "add" and "First".

While it initially seems like a verbose name, I think its probably the best we can do to balance consistency with the existing "addLogRecordProcessor" and the precedent in java with Deque#addFirst.

@onurkybsi onurkybsi changed the title SdkLoggerProviderBuilder. addLogRecordProcessorFirst added Apr 18, 2025
@jack-berg jack-berg merged commit 8fda429 into open-telemetry:main Apr 18, 2025
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants