Skip to content

fix: expand Array arguments in MarkerLoggingAdapter log templates (#3257)#3263

Merged
He-Pin merged 1 commit into
mainfrom
fix/3257-markerlogging-array-expand
Jun 28, 2026
Merged

fix: expand Array arguments in MarkerLoggingAdapter log templates (#3257)#3263
He-Pin merged 1 commit into
mainfrom
fix/3257-markerlogging-array-expand

Conversation

@He-Pin

@He-Pin He-Pin commented Jun 28, 2026

Copy link
Copy Markdown
Member

Motivation

MarkerLoggingAdapter.format1 passed an Array argument to the varargs
format(t, arg: Any*) without spreading it (: _*), so the whole array was
treated as a single template argument instead of being expanded into the
individual placeholders.

For example:

markerLog.info(LogMarker.Security, "{} {} {}", Array("a", "b", "c"))
base LoggingAdapter MarkerLoggingAdapter (before)
Output a b c ArraySeq(a, b, c) {} {}

The base LoggingAdapter.format1 expands arrays correctly (it calls
formatImpl(t, a.toSeq) directly). The marker copy exists for binary
compatibility reasons (it cannot reach the base trait's private formatImpl),
and when it was written the varargs spread (: _*) was forgotten. The scaladoc
promising that an Array argument "will be expanded into replacement
arguments" was therefore not honored by the marker adapter.

Fixes #3257.

Modification

Spread the array elements as varargs in both Array branches of
MarkerLoggingAdapter.format1, so each element fills its own placeholder, and
documented the reason so it is not regressed:

case a: Array[?] if !a.getClass.getComponentType.isPrimitive => format(t, a.toIndexedSeq: _*)
case a: Array[?]                                             => format(t, a.toIndexedSeq.asInstanceOf[IndexedSeq[AnyRef]]: _*)
case x                                                       => format(t, x)

DiagnosticMarkerBusLoggingAdapter inherits this method and is fixed by the
same change.

Result

  • Array arguments to the single-argument marker log template methods are now
    expanded into separate placeholders, matching LoggingAdapter and the
    documented behavior (e.g. "a b c" instead of "ArraySeq(a, b, c) {} {}").
  • format1 is private and only its body changed — no public API or
    binary-compatibility change
    (actor / mimaReportBinaryIssues is clean).

Tests

  • sbt "actor-tests/testOnly org.apache.pekko.event.MarkerLoggingSpec"
    all passed.
  • Added directional tests for a non-primitive Array[String] and a primitive
    Array[Int] (the two format1 branches). Both fail before the fix
    (rendering "ArraySeq(a, b, c) {} {}" / "ArraySeq(1, 2, 3) {} {}") and pass
    after.
  • sbt "actor/mimaReportBinaryIssues" → no binary issues.

References

Fixes #3257


This is an original contribution to Apache Pekko, made under the Apache License
2.0 (i.e. the changes are now Apache licensed). No third-party or Akka-derived
code is included.

Motivation:
MarkerLoggingAdapter.format1 passed an Array argument to the varargs format
without spreading it, so the whole array was treated as a single template
argument. A call like markerLog.info(marker, "{} {} {}", Array("a", "b", "c"))
rendered as "ArraySeq(a, b, c) {} {}" instead of "a b c". The base
LoggingAdapter expands arrays correctly; the marker copy did not.
DiagnosticMarkerBusLoggingAdapter inherits the same method and was affected too.

Modification:
- Spread the array elements as varargs in both Array branches of
  MarkerLoggingAdapter.format1 so each element fills its own placeholder.

Result:
Array arguments to the single-argument marker log template methods are now
expanded into separate placeholders, matching LoggingAdapter behavior.

Tests:
- actor-tests/testOnly org.apache.pekko.event.MarkerLoggingSpec - all passed.
  Added directional tests (non-primitive and primitive arrays) that fail
  before the fix (rendering ArraySeq(...)) and pass after.

References:
Fixes #3257
@pjfanning

Copy link
Copy Markdown
Member

Is there any chance that since the existing behaviour has been like that for a long time, that we just accept that that is the behaviour? Where in the docs does it say that this change is what users should expect?

@He-Pin

He-Pin commented Jun 28, 2026

Copy link
Copy Markdown
Member Author

The problem is this ,it change the default behavior.

private def format1(t: String, arg: Any): String = arg match {
case a: Array[?] if !a.getClass.getComponentType.isPrimitive => format(t, a.toIndexedSeq)
case a: Array[?] => format(t, a.toIndexedSeq.asInstanceOf[IndexedSeq[AnyRef]])
case a: Array[?] if !a.getClass.getComponentType.isPrimitive => format(t, a.toIndexedSeq: _*)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not sure if it's safe to change ,let's defer this.

@pjfanning

Copy link
Copy Markdown
Member

Actually, the javadoc says

 /**
   * Message template with 1 replacement argument.
   * The marker argument can be picked up by various logging frameworks such as slf4j to mark this log statement as "special".
   *
   * If `arg1` is an `Array` it will be expanded into replacement arguments, which is useful when
   * there are more than four arguments.
   * @see [[LoggingAdapter]]
   */
  def error(marker: LogMarker, cause: Throwable, template: String, arg1: Any): Unit =
    if (isErrorEnabled(marker)) bus.publish(Error(cause, logSource, logClass, format1(template, arg1), mdc, marker))

So your change is probably correct.

@He-Pin He-Pin marked this pull request as ready for review June 28, 2026 16:58
@He-Pin He-Pin added this to the 2.0.0-M4 milestone Jun 28, 2026
@He-Pin He-Pin added the bug Something isn't working label Jun 28, 2026

@pjfanning pjfanning left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@He-Pin He-Pin merged commit c0e9406 into main Jun 28, 2026
9 checks passed
@He-Pin He-Pin deleted the fix/3257-markerlogging-array-expand branch June 28, 2026 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MarkerLoggingAdapter.format1 does not expand Array arguments into template placeholders

2 participants