Skip to content

Improve rdump exception logging#167

Merged
yunzheng merged 15 commits intofox-it:mainfrom
JSCU-CNI:improvement/rdump-exception-handling
Jun 30, 2025
Merged

Improve rdump exception logging#167
yunzheng merged 15 commits intofox-it:mainfrom
JSCU-CNI:improvement/rdump-exception-handling

Conversation

@JSCU-CNI
Copy link
Contributor

@JSCU-CNI JSCU-CNI commented Mar 3, 2025

This PR aims to improve rdump's exception logging when using the Elastic adapter.

  1. We introduce an optional structlog dependency for colorized log output
  2. We now flatten Elastic API errors so they are visible to the end-user
  3. We now properly handle writer.excepthook so no exception occurs while handling exceptions
  4. Rdump now prints exceptions in a less verbose way unless extra verbosity is enabled.
  5. Return codes (0/1) of rdump should be handled better now.

Examples of new compact stacktraces:

$ target-query -q -f mft example.img | rdump -w "elastic+http://elastic:***@localhost:9200?verify_certs=0&index=test"
[reading from stdin]
2025-03-05T10:12:24.275042Z [error    ] rdump encountered a fatal error: Connection error caused by: ConnectionError(Connection error caused by: NewConnectionError(<urllib3.connection.HTTPConnection object at 0x7356a43ff4d0>: Failed to establish a new connection: [Errno 111] Connection refused))
$ 
$ rdump example.rec -w "elastic+http://elastic:***@localhost:9200?verify_certs=0&index=example"
[reading from stdin]
2025-03-05T10:16:45.148152Z [error    ] rdump encountered a fatal error: 500 document(s) failed to index. (400 strict_dynamic_mapping_exception [1:14] mapping set to strict, dynamic introduction of [hostname] within [_doc] is not allowed)
$

@codecov
Copy link

codecov bot commented Mar 3, 2025

Codecov Report

Attention: Patch coverage is 48.10127% with 41 lines in your changes missing coverage. Please review.

Project coverage is 82.24%. Comparing base (8fe4343) to head (f32f0eb).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
flow/record/adapter/elastic.py 35.48% 20 Missing ⚠️
flow/record/tools/rdump.py 62.50% 12 Missing ⚠️
flow/record/stream.py 25.00% 9 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #167      +/-   ##
==========================================
- Coverage   82.95%   82.24%   -0.72%     
==========================================
  Files          34       34              
  Lines        3591     3650      +59     
==========================================
+ Hits         2979     3002      +23     
- Misses        612      648      +36     
Flag Coverage Δ
unittests 82.24% <48.10%> (-0.72%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@JSCU-CNI JSCU-CNI requested a review from yunzheng March 5, 2025 15:32
@JSCU-CNI JSCU-CNI requested a review from yunzheng June 26, 2025 17:32
yunzheng
yunzheng previously approved these changes Jun 27, 2025
Copy link
Member

@yunzheng yunzheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, @Schamper do you also want to do a quick review?

Co-authored-by: Erik Schamper <1254028+Schamper@users.noreply.github.com>
@yunzheng yunzheng merged commit d842ba4 into fox-it:main Jun 30, 2025
23 of 25 checks passed
@JSCU-CNI JSCU-CNI deleted the improvement/rdump-exception-handling branch June 30, 2025 11:56
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.

3 participants