Skip to content

Default value for Syslog counting framing #48479

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

Merged
merged 1 commit into from
Jun 23, 2025
Merged

Conversation

mabartos
Copy link
Contributor

@mabartos mabartos commented Jun 19, 2025

The expected behavior described in the #48036 issue seems reasonable and also the suggested approach on how to achieve it described in #48036 (comment).

It should be in compliance with RFC-5425 and RFC-6587.

@dmlloyd Could you please check it? Thanks!

Closes quarkusio#48036

Signed-off-by: Martin Bartoš <mabartos@redhat.com>

This comment has been minimized.

@geoand geoand added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jun 22, 2025
Copy link

quarkus-bot bot commented Jun 22, 2025

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit f2e530c.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@geoand geoand merged commit de51d08 into quarkusio:main Jun 23, 2025
110 of 111 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.25 - main milestone Jun 23, 2025
@quarkus-bot quarkus-bot bot added kind/bugfix and removed triage/waiting-for-ci Ready to merge when CI successfully finishes labels Jun 23, 2025
@mabartos
Copy link
Contributor Author

@dmlloyd @geoand Would it be possible to backport it in 3.20?

@geoand
Copy link
Contributor

geoand commented Jun 23, 2025

That's a tricky one as this could theoretically break applications that (inadvertedly and incorrectly) depend on the previous behavior. Consider the following use case:

quarkus.log.syslog.use-counting-framing=true #this comment should not be here and results in the value being `false`

If we backport then this application will no longer build.

@mabartos
Copy link
Contributor Author

@geoand Thanks for the comment. Just thought that we could simply set the default value to false for the 3.20 backport and keeping the same functionality as before.

Consider the following use case:

I don't probably fully understand. If they explicitly specify the property, there should not be any change for them, right?

@geoand
Copy link
Contributor

geoand commented Jun 23, 2025

there should not be any change for them, right?

If they set the property in the way I showed in the hypothetical example above, with 3.20 they would effectively have quarkus.log.syslog.use-counting-framing=false while with this PR the application fails to start at all.
It's a super niche example, but as we are talking about LTS, I'd prefer to not backport unless there is a very good reason to do so.

@mabartos
Copy link
Contributor Author

@geoand Ahh, I didn't know about the issue with inline comments.

It's a super niche example, but as we are talking about LTS, I'd prefer to not backport unless there is a very good reason to do so.

Understandable; we'll probably keep the similar functionality on our side until we are on the next Quarkus LTS.

Thanks!

@geoand
Copy link
Contributor

geoand commented Jun 23, 2025

🙏🏽

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

Successfully merging this pull request may close these issues.

Default property values for syslog logging
4 participants