[EXPORTER] Fix Elasticsearch log exporter aborting on invalid UTF-8 - #4501
Open
om7057 wants to merge 2 commits into
Open
[EXPORTER] Fix Elasticsearch log exporter aborting on invalid UTF-8#4501om7057 wants to merge 2 commits into
om7057 wants to merge 2 commits into
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.22% 83.45% +0.23%
==========================================
Files 521 521
Lines 20404 20405 +1
==========================================
+ Hits 16980 17027 +47
+ Misses 3424 3378 -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.
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
1 participant
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).