Add missed standard types during batch's size estimation by andsel · Pull Request #19158 · elastic/logstash

andsel · 2026-05-22T14:17:34Z

Release notes

Fixed error during event size estimation on non recognised types.

What does this PR do?

Handle the missed Timestamp branch during the estimation.
Adds a couple of safe guards during estimation
1. if class is not belonging to the list of supported in Valufier (which should be a programming error) but it's serializable, use Java serialization
2. if such class is neither serilizable log a warning error
Updates the BigDecimal estimation considering the inner BigInteger instance

The safeguards log messages contains also the field path to the offending field.

Why is it important/What is the impact to the user?

Avoid to log error messages where the user can't take any action to solve and that doesn't impact event processing but just the size calculation.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
~~[ ] I have made corresponding changes to the documentation~~
~~[ ] I have made corresponding change to the default configuration files (and/or docker env variables)~~
I have added tests that prove my fix is effective or that my feature works

Author's Checklist

[ ]

How to test this PR locally

Test with the pipeline-to-pipeline as described in reproducer comment.

Related issues

Closes Batch metrics sampling fails when estimating memory for org.logstash.Timestamp type #19066

github-actions · 2026-05-22T14:17:45Z

🤖 GitHub comments

Just comment with:

run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)
run exhaustive tests : Run the exhaustive tests Buildkite pipeline.

mergify · 2026-05-22T14:19:09Z

This pull request does not have a backport label. Could you fix it @andsel? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit.
If no backport is necessary, please add the backport-skip label

…ertedMap

…ation

…ered the log level when an error reach the outer batch estimation method

…possible else throw an error

Copilot

Pull request overview

Fixes noisy error logging during pipeline batch-size estimation (issue #19066) caused by org.logstash.Timestamp values not being recognized by ConvertedMap.estimateMemory. Adds an explicit Timestamp branch, a Java-serialization fallback for unknown-but-Serializable types, threads a field path through the recursion to make diagnostic messages actionable, and downgrades the batch-metrics failure log from error to warn.

Changes:

Add a Timestamp branch and a Serializable fallback to ConvertedMap.estimateMemory, with field-path-enriched diagnostics.
Thread fieldPath through the recursive estimateMemory calls and update Event.estimateMemory to seed <data> / <meta> roots.
Downgrade the metric-failure log severity in QueueReadClientBatchMetrics and add unit/integration tests in ConvertedMapTest and EventTest.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
logstash-core/src/main/java/org/logstash/ConvertedMap.java	Adds Timestamp branch, Serializable fallback, and field-path-aware recursion (private signature changed to `(String, Object)`).
logstash-core/src/main/java/org/logstash/Event.java	Passes `<data>` / `<meta>` root field paths into `ConvertedMap.estimateMemory`.
logstash-core/src/main/java/org/logstash/execution/QueueReadClientBatchMetrics.java	Downgrades the catch-all log from error to warn.
logstash-core/src/test/java/org/logstash/ConvertedMapTest.java	Adds tests using `ListAppender` to cover serializable and non-serializable unknown types.
logstash-core/src/test/java/org/logstash/EventTest.java	Adds roundtrip estimateMemory test for nested `Timestamp` field.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…lure path

estolfo · 2026-05-28T09:35:04Z

Tested with the pipeline configuration in the comments of the linked issue and confirm I don't see the error and see the successful log lines estimating the batch metrics estimated memory consumption:

[2026-05-28T11:33:05,550][INFO ][org.logstash.execution.AbstractPipelineExt] Pipeline `upstream` batch metrics estimated memory consumption: 5191680 bytes
[2026-05-28T11:33:05,588][INFO ][org.logstash.execution.AbstractPipelineExt] Pipeline `downstream` batch metrics estimated memory consumption: 5191680 bytes

elasticmachine · 2026-05-28T14:26:43Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 5e5ab27

Failed CI Steps

:rspec: Ruby unit tests

History

💚 Build #4922 succeeded ba5bb3e
💚 Build #4921 succeeded 2d8c84d
💚 Build #4919 succeeded aa734ef
💛 Build #4909 was flaky dd45906
💛 Build #4908 was flaky f18db90
💛 Build #4906 was flaky f930ac4

cc @andsel

andsel · 2026-05-29T07:57:16Z

@Mergifyio backport 9.4

mergify · 2026-05-29T07:57:22Z

backport 9.4

✅ Backports have been created

Details

#19180 [9.4] (backport #19158) Add missed standard types during batch's size estimation has been created for branch 9.4

… estimation Fixed error during event size estimation on non recognised types. - Handle the missed `Timestamp` branch during the estimation. - Adds a couple of safe guards during estimation 1. if class is not belonging to the list of supported in Valufier (which should be a programming error) but it's serializable, use Java serialization 2. if such class is neither serilizable log a warning error - Updates the BigDecimal estimation considering the inner BigInteger instance The safeguards log messages contains also the field path to the offending field. (cherry picked from commit 0b09a83) Co-authored-by: Andrea Selva <selva.andre@gmail.com>

… estimation (#19179) * Add missed standard types during batch's size estimation (#19158) Fixed error during event size estimation on non recognised types. - Handle the missed `Timestamp` branch during the estimation. - Adds a couple of safe guards during estimation 1. if class is not belonging to the list of supported in Valufier (which should be a programming error) but it's serializable, use Java serialization 2. if such class is neither serilizable log a warning error - Updates the BigDecimal estimation considering the inner BigInteger instance The safeguards log messages contains also the field path to the offending field. (cherry picked from commit 0b09a83) # Conflicts: # logstash-core/src/main/java/org/logstash/ConvertedMap.java * [git conflict] Fixed * Fixed API invocation, instead of use JRuby 10.0 use the 9.4 version --------- Co-authored-by: Andrea Selva <selva.andre@gmail.com>

andsel self-assigned this May 22, 2026

andsel force-pushed the fix/avoid_type_errors_in_batch_estimate branch from f18db90 to dd45906 Compare May 26, 2026 15:24

andsel added the bug label May 28, 2026

andsel marked this pull request as ready for review May 28, 2026 08:11

andsel added 6 commits May 28, 2026 10:11

Small test to verify the ability to have a Timestamp in a nested Conv…

fed7a4f

…ertedMap

Added Timestamp to the list of considered type during estimate compur…

28d842e

…ation

[Config] Removed test config definitions

33270a5

[Cleanup] removed unused package imports

4a562d8

[Impl] Updated batch size estimation to use Jva serialization and low…

ffd9d05

…ered the log level when an error reach the outer batch estimation method

[Feature] at last resort try to estimate using Java serialization if …

11ed019

…possible else throw an error

andsel force-pushed the fix/avoid_type_errors_in_batch_estimate branch from dd45906 to 11ed019 Compare May 28, 2026 08:11

andsel requested a review from Copilot May 28, 2026 08:14

Copilot started reviewing on behalf of andsel May 28, 2026 08:15 View session

Copilot AI reviewed May 28, 2026

View reviewed changes

andsel added 3 commits May 28, 2026 10:27

[Fix] invoke the right estimate recursive method for RubySymbol

adf3a06

[Test] fixed typos in tests

3788639

[Test] tightened assertion on exception type to make it clear the fai…

aa734ef

…lure path

estolfo suggested changes May 28, 2026

View reviewed changes

Comment thread logstash-core/src/main/java/org/logstash/ConvertedMap.java

Comment thread logstash-core/src/main/java/org/logstash/ConvertedMap.java Outdated

Comment thread logstash-core/src/main/java/org/logstash/ConvertedMap.java Outdated

andsel added 2 commits May 28, 2026 13:40

[Fix] fixed size for RubyFixNum and added null check

2d8c84d

Reworked the BigDecimal estimation given that uses BigInteger internally

ba5bb3e

estolfo approved these changes May 28, 2026

View reviewed changes

[skip test] expanded * imports

5e5ab27

andsel merged commit 0b09a83 into elastic:main May 28, 2026
11 checks passed

andsel added backport-9.3 Automated backport to the 9.3 branch backport-9.4 labels May 29, 2026

mergify Bot mentioned this pull request May 29, 2026

[9.3] (backport #19158) Add missed standard types during batch's size estimation #19179

Merged

mergify Bot mentioned this pull request May 29, 2026

[9.4] (backport #19158) Add missed standard types during batch's size estimation #19180

Merged

3 tasks

Navigation Menu

Uh oh!

Add missed standard types during batch's size estimation - #19158

Add missed standard types during batch's size estimation#19158
andsel merged 12 commits into
elastic:mainfrom
andsel:fix/avoid_type_errors_in_batch_estimate

andsel commented May 22, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 22, 2026

Uh oh!

mergify Bot commented May 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

estolfo commented May 28, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elasticmachine commented May 28, 2026

Uh oh!

Uh oh!

andsel commented May 29, 2026

Uh oh!

mergify Bot commented May 29, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

andsel commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Release notes

What does this PR do?

Why is it important/What is the impact to the user?

Checklist

Author's Checklist

How to test this PR locally

Related issues

Uh oh!

github-actions Bot commented May 22, 2026

🤖 GitHub comments

Uh oh!

mergify Bot commented May 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

estolfo commented May 28, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elasticmachine commented May 28, 2026

💛 Build succeeded, but was flaky

Failed CI Steps

History

Uh oh!

Uh oh!

andsel commented May 29, 2026

Uh oh!

mergify Bot commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Backports have been created

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

andsel commented May 22, 2026 •

edited

Loading

mergify Bot commented May 29, 2026 •

edited

Loading