Removing StructuredDataMessage and all other RFC 5424 et al.

[vy]

I propose removing

o.a.l.l.message.StructuredDataMessage
o.a.l.l.message.StructuredDataId
o.a.l.l.message.StructuredDataCollectionMessage
o.a.l.l.core.layout.SyslogLayout
o.a.l.l.core.appender.SyslogAppender
o.a.l.l.core.layout.Rfc5424Layout

because

• They're not used. (Corpus search revealed no programmatic access¹.)
• They, if not incorrect, insufficiently model the RFCs they address.
• Associated layouts are not really structured. Their structure guarantee is amended with several exceptions.
• They have several bugs.
• They receive vulnerability reports.
• It has been the biggest maintenance burden of the last 6 months.

If you agree and/or disagree with this proposal, please share your thoughts with your reasoning.

¹ I've searched a corpus of 706,389 unique Maven coordinates, amounting to 1,288,939 artifacts (the link is only accessible by ASF and Logging Services PMC members!) and 238,544,576 classes in total, and spotted no usages.

[ppkarwasz]

What is your definition of a structured layout?

We already agreed that BSD syslog is not structured: it is a set of loose conventions that every implementation applies slightly differently.

But I do not follow why RFC 5424 fails the bar. If the reason is that it imposes length and syntax restrictions (on SD-ID, PARAM-NAME, and friends), then XML fails by the exact same standard: element names have their own syntax restrictions, and the XML layouts had the very same gap on non-printable characters.

I have no objection to removing these classes in a major release. What gives me pause is that this reads like a removal in 2.x, because there is no 3.x line for the Log4j API. That changes the calculus:

1. Usage is poor, but not zero. This would, for instance, break OpenTelemetry, which builds on these classes directly (see LogEventToReplay). The question is not how many people call StructuredDataMessage, but whether breaking the ones who do is worth it in a minor release.

2. Everything gets security reports. The "I found Log4Shell in my garage" cluster is comparable in size to the RFC 5424 cluster and the XML cluster, and a large part of it exists only because we did not release fast enough and collected duplicates. A few highlights from the last six months:

   • "Apache Log4j 2.x — Infinite Recursion Leading to Denial of Service (CVE-2021-45105)": a report rediscovering CVE-2021-45105, which it cites by number, roughly four years late.
   • "Log4Shell primitive (JNDI lookup → outbound LDAP) with SSRF/Exfil evidence": breaking news that Log4Shell can also be pointed at internal hosts.
   • "Remote log4net configuration via URI leads to SSRF and remote code execution": I genuinely cannot reconstruct what this one was claiming.
   • "log4j-docker DockerLookup performs unvalidated SSRF via DOCKER_URI environment variable — AWS IMDSv1 credential theft confirmed": an attacker who already controls the DOCKER_URI environment variable confirms they can reach an endpoint.
   • "Log4j Configuration Processing Vulnerability Report" and "Log4j Lookup Vulnerability Report": two near-content-free filings against Interpolator/StrSubstitutor, both self-rated High.

   None of these are RFC 5424, and removing RFC 5424 would not have stopped a single one of them.

3. The actual fix cost has been modest. Over the past three months we shipped fixes for several real issues. For RFC 5424 I wrote two patches: far less code than the XML layouts needed, and certainly easier than the verifyHostName tests.

TL;DR: I would recommend moderation. The real question is not the user count, it is whether the maintenance we are actually spending on these classes justifies breaking the usages that exist, in a minor release. How many problems do you realistically expect to keep finding in them?

[vy]

I would recommend moderation.

@ppkarwasz, I am not able to follow. What is "moderation"? What are you suggesting?

[vy]

The actual fix cost has been modest.

@ppkarwasz, right, but triage is not. ASF employs salaried staff (including you) to help with triage. Dealing with incoming vulnerability report submissions (their triage and, if necessary fix) is NOT modest.

[ppkarwasz]

We can for example:

• Mark those classes as deprecated for removal.
• Remove the layouts in Log4j 3.x or move them to a separate artifact.
• Get in contact with some modern syslog users and propose to integrate the message implementations in their library: https://github.com/teragrep/jla_06

[FreeAndNil]

@ppkarwasz this is the path I would prefer.

[garydgregory]

Please. Don't. Break. Anything. In 2.x. PLEASE. Sigh.

[rgoers]

I would suggest fixing StructuredDataMessage. First, add a StructuredData element that contains the SD-ID and the key/value pairs. Then modify StructuredDataMessage with a constructor that accepts List of StructuredDataElements and a String message. I would deprecate the existing constructors in 2.x and remove them in 3.x. I would then modify StructuredDataMessage accordingliy.

Since StructuredDataMessage should extend MapMessage I would suggest that when acting as a MapMessage the maps of the StructuredDataMessages be merged and their keys be prefixed with the SD-ID value followed by ":". The message should be stored in a key with the value "message" in the MapMessage map. I would then modify RFC5424Layout to leverage this structure. Everything else should treat it as a MapMessage.

StructuredDataCollectionMessage should be deprecated in 2.x and removed in 3.x. It makes no sense.

[jvz]

I would love to use this API to make structured logging calls for JSON layouts as well. If this type of message were useful for cases where logs may go to either syslog or some other system, I'd like to be able to use more structure in the log message than free form text. This makes parsing logs much easier.

[rgoers]

What would StucturedDataMessage get you that MapMessage does not?

[ramanathan1504]

I've been looking closely at our roadmap and how this proposal to clean up legacy RFC 5424 / StructuredDataMessage components might intersect with other open issues.

If we do proceed with removing these legacy pieces, perhaps we can consider it a great stepping stone towards the goals outlined in Issue #1976.

Replacing these older, high-maintenance components with native support for W3C Trace Context standards (traceId, spanId, traceFlags) directly on LogEvent would give us a highly performant, modern alternative. As discussed in #1976, this would bypass the memory and GC overhead of copying ThreadContext maps while keeping Log4j aligned with cloud-native standards.

I wanted to share this thought to see if coordinating these two topics is a direction we want to explore for Log4j 3.x. If the team agrees this is a good path forward.

[rgoers]

I am not sure I follow that. The ThreadContextMap is used for more than just tracing. But that is a discussion best left to its own issue. The only thing that remotely connects that issue with this one is the phrase "clean up" and even that is misleading since 1976 doesn't "clean up" anything on its own.

[ramanathan1504]

That makes complete sense. You are absolutely right that ThreadContextMap has a much broader scope and that #1976 is a feature addition rather than a cleanup.

I agree it is best to keep this discussion focused strictly on the removal of the RFC 5424 components. I will follow up on #1976 directly for any tracing and metadata discussions. Thank you for the clarification!