Skip to content

Conversation

keynslug
Copy link
Contributor

@keynslug keynslug commented Jul 23, 2025

Fixes EMQX-14467.

Release version: 6.0.0

Summary

This PR exposes trace file size limit as a configuration option, with 512 MiB preserved as a default.

Now that it's possible for the user to set much lower limits, risk of hitting this limit is increased. Currently, because tracing is implemented on top of logger subsystem components, there's no direct feedback: active trace does not know that the limit is hit. Instead, messages like these are printed to stdout / stderr:

Logger - error: {disk_log,'trace_topic_CLI-trace_max_file_size','trace_topic_CLI-trace_max_file_size',full}
Logger - error: {'trace_topic_CLI-trace_max_file_size',log,#{type=>halt,file=>"tmp/trace_max_file_size.log",max_no_bytes=>5120,max_no_files=>undefined},{error,{full,'trace_topic_CLI-trace_max_file_size'}}}

This is obviously poor user experience, improvements to be made in followup PRs. Proposals are outlined here.

PR Checklist

  • For internal contributor: there is a jira ticket to track this change
  • The changes are covered with new or existing tests
  • Change log for changes visible by users has been added to changes/ee/(feat|perf|fix|breaking)-<PR-id>.en.md files
  • Schema changes are backward compatible

@keynslug keynslug marked this pull request as ready for review July 24, 2025 11:24
@keynslug keynslug requested a review from a team as a code owner July 24, 2025 11:24
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Project coverage is 84.43%. Comparing base (e390716) to head (13e7c94).
Report is 63 commits behind head on release-60.

Files with missing lines Patch % Lines
apps/emqx/src/emqx_schema.erl 75.00% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff               @@
##           release-60   #15556      +/-   ##
==============================================
+ Coverage       81.66%   84.43%   +2.77%     
==============================================
  Files            1127     1114      -13     
  Lines           82581    78686    -3895     
==============================================
- Hits            67438    66439     -999     
+ Misses          15143    12247    -2896     

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

?assertMatch(
FS when FS =< MaxSize andalso FS > MaxSize div 2,
FileSize,
{max_file_size, MaxSize}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: maybe add FileSize too to have more context.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be part of {actual, ...} tuple if match fails.

@keynslug keynslug merged commit b287a4b into emqx:release-60 Jul 24, 2025
290 of 292 checks passed
@keynslug keynslug deleted the feat/EMQX-14467/config branch July 24, 2025 15:12
@emqxqa
Copy link

emqxqa commented Jul 28, 2025

TestExecution

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.

4 participants