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

mqtt: enable limiting of logged message length #11053

Closed
wants to merge 2 commits into from

Conversation

satta
Copy link
Contributor

@satta satta commented May 10, 2024

Link to redmine ticket: https://redmine.openinfosecfoundation.org/issues/6984

Describe changes:

  • Add an msg-log-limit option to enable optional truncation of the message payloads in MQTT.
SV_BRANCH=https://github.com/OISF/suricata-verify/pull/1828

@satta satta requested review from jasonish, victorjulien and a team as code owners May 10, 2024 22:47
Copy link

codecov bot commented May 10, 2024

Codecov Report

Attention: Patch coverage is 93.05556% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 83.65%. Comparing base (abb7424) to head (5edd982).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11053      +/-   ##
==========================================
+ Coverage   80.63%   83.65%   +3.01%     
==========================================
  Files         922      922              
  Lines      250137   250338     +201     
==========================================
+ Hits       201699   209417    +7718     
+ Misses      48438    40921    -7517     
Flag Coverage Δ
fuzzcorpus 64.28% <33.33%> (+0.01%) ⬆️
livemode 18.42% <13.33%> (-0.13%) ⬇️
suricata-verify 62.76% <83.33%> (?)
unittests 62.26% <62.50%> (-0.01%) ⬇️

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

Copy link
Contributor

@jlucovsky jlucovsky left a comment

Choose a reason for hiding this comment

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

Please add a documentation update to doc/userguide/configuration/suricata-yaml.rst

@satta
Copy link
Contributor Author

satta commented May 12, 2024

Hmm, it looks like the included file doc/userguide/partials/eve-log.yaml hasn't been updated in some time. I guess that's where you meant the additional options to be listed? Should I bring it up to date with the current state of supported event types?

Also, I don't think that the logging options should go into doc/userguide/configuration/suricata-yaml.rst? The app-layer parser configuration is there, but the changes here only affect logging. I think it would probably be best to add a MQTT section to doc/userguide/output/eve/eve-json-output.rst. What do you think?

@satta
Copy link
Contributor Author

satta commented May 12, 2024

New PR: #11054 11054

@satta satta closed this May 12, 2024
@catenacyber
Copy link
Contributor

Also, I don't think that the logging options should go into doc/userguide/configuration/suricata-yaml.rst? The app-layer parser configuration is there, but the changes here only affect logging. I think it would probably be best to add a MQTT section to doc/userguide/output/eve/eve-json-output.rst. What do you think?

I think doc/userguide/configuration/suricata-yaml.rst is meant to document everything in suricata.yaml
@jufajardini what do you think ?

@jufajardini
Copy link
Contributor

Also, I don't think that the logging options should go into doc/userguide/configuration/suricata-yaml.rst? The app-layer parser configuration is there, but the changes here only affect logging. I think it would probably be best to add a MQTT section to doc/userguide/output/eve/eve-json-output.rst. What do you think?

I think doc/userguide/configuration/suricata-yaml.rst is meant to document everything in suricata.yaml @jufajardini what do you think ?

I see that Sascha has addressed this, but just to answer - it seems to be what we're following, indeed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants