Skip to content

fix: correct condition for recording successful log processing metrics#8226

Merged
jack-berg merged 1 commit intoopen-telemetry:mainfrom
TruongNguyen1305:fix/log-record-processor/finish-logs
Mar 30, 2026
Merged

fix: correct condition for recording successful log processing metrics#8226
jack-berg merged 1 commit intoopen-telemetry:mainfrom
TruongNguyen1305:fix/log-record-processor/finish-logs

Conversation

@TruongNguyen1305
Copy link
Copy Markdown
Contributor

Description

This PR corrects a logical condition in LegacyLogRecordProcessorInstrumentation.finishLogs.

Problem

Previously, the code recorded metrics under the comment:

Legacy metrics only record when no error.

However, the implementation used:

if (error != null) {
    processedLogs().add(count, standardAttrs);
}

This records metrics when an error is present, which is inconsistent with the stated intent.

Fix

Updated the condition to:

if (error == null) {
    processedLogs().add(count, standardAttrs);
}

@TruongNguyen1305 TruongNguyen1305 requested a review from a team as a code owner March 30, 2026 18:08
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla bot commented Mar 30, 2026

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: TruongNguyen1305 / name: Nguyen Dang Truong (1f92cc6)

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.33%. Comparing base (d688fa6) to head (1f92cc6).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8226      +/-   ##
============================================
+ Coverage     90.31%   90.33%   +0.02%     
- Complexity     7652     7654       +2     
============================================
  Files           843      843              
  Lines         23071    23071              
  Branches       2311     2311              
============================================
+ Hits          20836    20841       +5     
+ Misses         1516     1513       -3     
+ Partials        719      717       -2     

☔ 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.

Copy link
Copy Markdown
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.

Should have looked for other instances of similar bugs and found this with #8145.

I did a check this time and don't see any other instances of this shape of bug.

Thanks!

@jack-berg jack-berg merged commit e40dca3 into open-telemetry:main Mar 30, 2026
27 checks passed
@otelbot
Copy link
Copy Markdown
Contributor

otelbot bot commented Mar 30, 2026

Thank you for your contribution @TruongNguyen1305! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants