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

QQE-673 | Cover max-length option for syslog logs #1839

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

fedinskiy
Copy link
Contributor

@fedinskiy fedinskiy commented Jun 13, 2024

Adds new module for working with syslogs, since jboss uses json logs which have less predictable size.
Uses official image of sylog-ng. Syslog-ng was chose over rsyslog and logstash because it supports syslog input and stdout output out of the box, without additional plugins.

Required for https://issues.redhat.com/browse/QUARKUS-4531

Summary

(Summarize the problem solved by this PR, and how to verify it manually)

Please select the relevant options.

  • Bug fix (non-breaking change which fixes an issue)
  • Dependency update
  • Refactoring
  • Backport
  • New scenario (non-breaking change which adds functionality)
  • This change requires a documentation update
  • This change requires execution against OCP (use run tests phrase in comment)

Checklist:

  • Methods and classes used in PR scenarios are meaningful
  • Commits are well encapsulated and follow the best practices

@michalvavrik
Copy link
Member

@fedinskiy please link TP

Copy link
Member

@michalvavrik michalvavrik left a comment

Choose a reason for hiding this comment

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

I think it looks good, I'd like to check TP, see CI green and have very minor comments addressed. Then we can get it in. Thanks

@fedinskiy fedinskiy force-pushed the feature/syslog-max-length branch from 00da183 to baf13f6 Compare June 13, 2024 11:36
@fedinskiy
Copy link
Contributor Author

This is a coverage for a single issue[1], so there is no TP. The new module was added only because we do not have a proper catch-all logging module

[1] quarkusio/quarkus@43b0f5d

Copy link
Member

@michalvavrik michalvavrik left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@michalvavrik
Copy link
Member

This is a coverage for a single issue[1], so there is no TP. The new module was added only because we do not have a proper catch-all logging module

[1] quarkusio/quarkus@43b0f5d

I can see that ticket is feature https://issues.redhat.com/browse/QUARKUS-4531, that's why I asked for the TP. Personally I don't mind, let's not create TP just because of me. Changes LGTM.

@fedinskiy fedinskiy force-pushed the feature/syslog-max-length branch from baf13f6 to 983d9e5 Compare June 13, 2024 11:53
@michalvavrik
Copy link
Member

FYI @fedinskiy validation fails

Adds new module for working with syslogs, since jboss uses json logs
which have less predictable size.
Uses official image pof sylog-ng. Syslog-ng was chose over rsyslog and
logstash because it supports syslog input and stdout output out of the
box, without additional plugins.

Required for https://issues.redhat.com/browse/QUARKUS-4531
@fedinskiy fedinskiy force-pushed the feature/syslog-max-length branch from 983d9e5 to 36a1dd9 Compare June 13, 2024 15:20
@michalvavrik
Copy link
Member

SyslogIT is passing, failures unrelated, merging

@michalvavrik michalvavrik merged commit 88ab570 into quarkus-qe:main Jun 13, 2024
7 of 9 checks passed
@jedla97 jedla97 added this to the 2.7 milestone Jun 20, 2024
@jedla97 jedla97 removed this from the 2.7 milestone Jun 20, 2024
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