[EXPORTER] Fix Elasticsearch log exporter aborting on invalid UTF-8 - #4501
Merged
marcalff merged 3 commits intoSep 1, 2026
Merged
Conversation
A log record's body or attributes may carry bytes that are not valid UTF-8 (e.g. a truncated multibyte sequence, a payload read in another encoding, or a string built from a file path). ElasticSearchRecordable::WriteValue stores the value as given, without validating or transcoding it. Export() then called nlohmann::json::dump() with its default strict error handler, which throws on invalid UTF-8; since Export() is noexcept, that throw became std::terminate() and aborted the whole process, not just the one export. Switch to error_handler_t::replace, which substitutes U+FFFD for the invalid bytes instead of throwing, so the record (and the rest of the batch) is still exported. Fixes open-telemetry#4439
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4501 +/- ##
==========================================
+ Coverage 83.25% 83.47% +0.23%
==========================================
Files 521 521
Lines 20379 20380 +1
==========================================
+ Hits 16964 17011 +47
+ Misses 3415 3369 -46
🚀 New features to boost your workflow:
|
…ctor> The fake Response's ForEachHeader overrides use nostd::function_ref directly; it was only reaching the test via a transitive include. <memory> and <vector> are only used transitively too and iwyu wants them dropped.
marcalff
reviewed
Sep 1, 2026
Comment on lines
+130
to
+134
| // Regression test: a log record whose body carries bytes that are not valid UTF-8 used to | ||
| // abort the process. ElasticSearchRecordable::WriteValue stores the value as given, and | ||
| // Export() previously called nlohmann::json::dump() with its default strict error handler, | ||
| // which throws on invalid UTF-8; since Export() is noexcept, that throw became | ||
| // std::terminate(). The exporter now tolerates it instead of crashing. |
Member
There was a problem hiding this comment.
No changes needed, but a nit comment for future PRs:
Comments in the CODE should be about what the code does.
Comments about what the code did before a change belongs to the PR or issue description, not in the code itself.
That said, thanks for the tests.
marcalff
approved these changes
Sep 1, 2026
marcalff
left a comment
marcalff
left a comment
Member
There was a problem hiding this comment.
LGTM, thanks for the fix and tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4439
Summary
A log record's body or attributes may carry bytes that are not valid UTF-8 (a truncated multibyte sequence, a payload read in another encoding, a message built from a file path, etc).
ElasticSearchRecordable::WriteValuestores the value as given, without validating or transcoding it.Export()then callednlohmann::json::dump()with its default strict error handler, which throwstype_error.316on invalid UTF-8. SinceExport()isnoexcept, that throw becomesstd::terminate(), aborting the whole process rather than just failing the one export.Fix
Switch to
error_handler_t::replace, which substitutes U+FFFD for the invalid bytes instead of throwing. The record (and the rest of the bulk batch) is still exported, with just the invalid bytes replaced.Test plan
ElasticsearchLogsExporterTests.ExportingARecordWithInvalidUtf8DoesNotAbort, using a fake injectableHttpClient/Session/Request/Response(the exporter already supports client injection) so the test doesn't need a real Elasticsearch instance.std::terminate/abort from the issue.es_log_record_exporter_testsuite passes (3 tests, 3 pre-existing disabled tests unaffected).