Skip to content

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

Merged
jack-berg merged 1 commit into
open-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 into
open-telemetry:mainfrom
TruongNguyen1305:fix/log-record-processor/finish-logs

Conversation

@TruongNguyen1305

@TruongNguyen1305 TruongNguyen1305 commented Mar 30, 2026

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

linux-foundation-easycla Bot commented Mar 30, 2026 •
edited
Loading

Copy link
Copy Markdown

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

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

@codecov

codecov Bot commented Mar 30, 2026 •
edited
Loading

Copy link
Copy Markdown

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.

jack-berg
jack-berg approved these changes Mar 30, 2026
View reviewed changes

@jack-berg jack-berg left a comment

Copy link
Copy Markdown
Member

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

otelbot Bot commented Mar 30, 2026

Copy link
Copy Markdown
Contributor

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

Reviewers

@jack-berg jack-berg jack-berg approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@TruongNguyen1305 @jack-berg