Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0217 improve frame too large logging context #12530

Merged

Conversation

zmstone
Copy link
Member

@zmstone zmstone commented Feb 18, 2024

Release version: v/e5.6.0

Summary

  • Log parsed bytes and bytes limit for frame_too_large error
  • Refactored connect packet parse failures to include more details

example trace:

2024-02-18T19:40:28.241495+01:00 [MQTT] test@127.0.0.1:47552 msg: mqtt_packet_received, packet: {frame_error,#{limit => 100,cause => frame_too_large,received => 143}}
2024-02-18T19:40:28.241765+01:00 [SOCKET] test@127.0.0.1:47552 msg: socket_force_closed, reason: frame_too_large
2024-02-18T19:40:28.242150+01:00 [SOCKET] test@127.0.0.1:47552 msg: emqx_connection_terminated, reason: {shutdown,frame_too_large}

PR Checklist

Please convert it to a draft if any of the following conditions are not met. Reviewers may skip over until all the items are checked:

  • Added tests for the changes
  • Added property-based tests for code which performs user input validation
  • Changed lines covered in coverage report
  • Change log has been added to changes/(ce|ee)/(feat|perf|fix|breaking)-<PR-id>.en.md files
  • For internal contributor: there is a jira ticket to track this change
  • Created PR to emqx-docs if documentation update is required, or link to a follow-up jira ticket
  • Schema changes are backward compatible

Checklist for CI (.github/workflows) changes

  • If changed package build workflow, pass this action (manual trigger)
  • Change log has been added to changes/ dir for user-facing artifacts update

@zmstone zmstone requested review from lafirest and a team as code owners February 18, 2024 18:28
@zmstone zmstone mentioned this pull request Feb 18, 2024
'reason' is maybe the wrapping field's name, so it was not used.
'hint' however, per our logging convention, is usually a free text
description for human to read.
change to 'cause' here because the field is always an atom and
it's use as shutdown counter in esockd_connection_sup
@zmstone zmstone force-pushed the 0217-improve-frame-too-large-logging-context branch from accf94b to 8b0e15e Compare February 19, 2024 08:38
@@ -0,0 +1 @@
Enhanced `frame_too_large` and malformed CONNECT packet parse failures to include more information to help troubleshooting.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Enhanced `frame_too_large` and malformed CONNECT packet parse failures to include more information to help troubleshooting.
Enhanced `frame_too_large` and malformed CONNECT packet error logs to include more information to help troubleshooting.

Otherwise it may sound like the PR has improved failures.

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks. will fix in the next commit to come

@zmstone zmstone merged commit 866fea7 into emqx:master Feb 19, 2024
167 checks passed
@zmstone zmstone deleted the 0217-improve-frame-too-large-logging-context branch February 19, 2024 10:33
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.

None yet

2 participants