镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
/dd-java-agent/cws-tls/ @DataDog/apm-java
/dd-java-agent/load-generator/ @DataDog/apm-java

/dd-java-agent/agent-bootstrap/src/**/datadog/trace/bootstrap/instrumentation/log/LogPatternInjector* @DataDog/apm-java

# Shared trace protocol
/utils/test-agent-utils/ @DataDog/apm-java @DataDog/apm-lang-platform-java @DataDog/apm-sdk-capabilities
/dd-trace-api/src/**/*ProtocolVersion* @DataDog/apm-java @DataDog/apm-lang-platform-java @DataDog/apm-sdk-capabilities
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,237 @@
package datadog.trace.bootstrap.instrumentation.log;

/** Adds missing MDC converters after the first complete field of a text logging pattern. */
public final class LogPatternInjector {
public static final int SERVICE = 1;
public static final int TRACE_ID = 2;
public static final int SPAN_ID = 4;
public static final int ALL = SERVICE | TRACE_ID | SPAN_ID;

private LogPatternInjector() {}

/** Uses fields discovered by Logback's native parser. */
public static String injectLogback(String pattern, int presentFields) {
return inject(pattern, presentFields, true);
}

/** MDC syntax capabilities of the supported Log4j releases. */
public enum Format {
LOG4J2_LEGACY(false),
LOG4J2(true);

private final boolean keyLists;

Format(boolean keyLists) {
this.keyLists = keyLists;
}
}

/** Recognizes MDC converters using the syntax supported by the running framework. */
public static String injectLog4j(String pattern, Format format) {
if (pattern == null || pattern.isEmpty()) {
return pattern;
}
return inject(pattern, fields(pattern, 0, pattern.length(), format), false);
}

private static int field(String key) {
if ("dd.service".equals(key)) {
return SERVICE;
}
if ("dd.trace_id".equals(key)) {
return TRACE_ID;
}
return "dd.span_id".equals(key) ? SPAN_ID : 0;
}

private static int fields(String pattern, int start, int end, Format format) {
int found = 0;
for (int i = start; i < end; i++) {
if (pattern.charAt(i) != '%') {
continue;
}
i++;
if (i < end && pattern.charAt(i) == '%') {
continue;
}
while (i < end
&& (pattern.charAt(i) == '-'
|| pattern.charAt(i) == '.'
|| Character.isDigit(pattern.charAt(i)))) {
i++;
}
int nameStart = i;
while (i < end && Character.isLetter(pattern.charAt(i))) {
i++;
}
String name = pattern.substring(nameStart, i);
boolean mdc = "X".equals(name) || "mdc".equals(name) || "MDC".equals(name);
boolean firstOption = true;
if (mdc && (i == end || pattern.charAt(i) != '{')) {
return ALL;
}
while (i < end && pattern.charAt(i) == '{') {
int close = optionEnd(pattern, i, end);
if (close < 0) {
return ALL; // Leave malformed patterns to the framework's validation.
}
if (mdc && firstOption) {
String keys = pattern.substring(i + 1, close);
if (format.keyLists && keys.indexOf(',') > 0) {
for (String key : keys.split(",")) {
found |= field(key.trim());
}
} else {
found |= field(keys);
}
} else if (firstOption && isComposite(name)) {
found |= fields(pattern, i + 1, close, format);
}
firstOption = false;
i = close + 1;
}
i--;
}
return found;
}

private static boolean isComposite(String name) {
return "style".equals(name)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Recognize MDC converters inside built-in Log4j2 wrappers

The composite list omits Log4j2's built-in color wrappers and maxLength alias, so existing MDC converters inside them are missed. For example, %cyan{%X{dd.trace_id}} %m%n gains a duplicate trace ID, while %blue{%X} %m%n gains all three fields despite already printing the entire MDC. Recognize these wrappers to preserve the promise to add only missing fields.

Suggested change
return "style".equals(name)
return "black".equals(name)
|| "blue".equals(name)
|| "cyan".equals(name)
|| "green".equals(name)
|| "magenta".equals(name)
|| "red".equals(name)
|| "white".equals(name)
|| "yellow".equals(name)
|| "maxLength".equals(name)
|| "style".equals(name)

React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it

What each reaction means
  • 👍 Useful: correct, and you're fixing it or will.
  • 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
  • 😕 Unclear: you can't tell what it means or why it matters.
  • 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.

🤖 Bits Code Review · Open Bits AI session

|| "highlight".equals(name)
Comment on lines +98 to +100

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Recognize all Log4j composite converter aliases

When a supported Log4j pattern wraps an existing correlation converter in %upperCase{...}, %lowerCase{...}, or the maxLength alias, fields() only descends into names accepted by this incomplete allowlist. It therefore fails to mark the nested key and injects a second converter (for %upperCase{%X{dd.trace_id}}, the ID is printed twice), so include all Log4j composite names and aliases.

Useful? React with 👍 / 👎.

|| "replace".equals(name)
|| "notEmpty".equals(name)
|| "varsNotEmpty".equals(name)
|| "variablesNotEmpty".equals(name)
|| "equals".equals(name)
|| "equalsIgnoreCase".equals(name)
|| "maxLen".equals(name)
|| "enc".equals(name)
|| "encode".equals(name);
}

private static int optionEnd(String pattern, int start, int end) {
int depth = 1;
for (int i = start + 1; i < end; i++) {
char c = pattern.charAt(i);
if (c == '{') {
depth++;
} else if (c == '}' && --depth == 0) {
return i;
}
}
return -1;
}

private static boolean isLineBreak(String pattern, int i, boolean logback) {
return pattern.startsWith("%n", i)
&& (logback || i + 2 == pattern.length() || !Character.isLetter(pattern.charAt(i + 2)));
}

private static String inject(String pattern, int presentFields, boolean logback) {
if (pattern == null || pattern.isEmpty() || presentFields == ALL) {
return pattern;
}
// Preserve the first complete field, including date options and presentation wrappers.
int insertion = pattern.length();
int optionsDepth = 0;
int compositeDepth = 0;
int bracketDepth = 0;
char quote = 0;
boolean fieldStarted = false;
for (int i = 0; i < pattern.length(); i++) {
char c = pattern.charAt(i);
if (c == '\\' && logback) {
fieldStarted = true;
i++;
continue;
}
if (!logback && c == '%' && i + 1 < pattern.length() && pattern.charAt(i + 1) == '%') {
fieldStarted = true;
i++;
continue;
}
if (optionsDepth > 0) {
if (quote != 0) {
if (c == quote) {
quote = 0;
}
} else if (c == '\'' || c == '"') {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Use framework-specific quoting rules for the insertion point

With injection enabled, the valid Log4j2 pattern %replace{%m}{'}{}%n acquires correlation fields after its newline. This scanner incorrectly treats the apostrophe as opening quoted text, although Log4j2 treats it as a literal regex character. The message and correlation fields end up on separate lines, with the fields running into the next event. Restrict this quote handling to Logback.

Suggested change
} else if (c == '\'' || c == '"') {
} else if (logback && (c == '\'' || c == '"')) {

React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it

What each reaction means
  • 👍 Useful: correct, and you're fixing it or will.
  • 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
  • 😕 Unclear: you can't tell what it means or why it matters.
  • 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.

🤖 Bits Code Review · Open Bits AI session

quote = c;
} else if (c == '{') {
optionsDepth++;
} else if (c == '}') {
optionsDepth--;
}
continue;
}
if (c == '{') {
optionsDepth++;
} else if (logback && c == '(') {
compositeDepth++;
} else if (logback && c == ')' && compositeDepth > 0) {
compositeDepth--;
} else if (compositeDepth == 0 && c == '[') {
bracketDepth++;
} else if (compositeDepth == 0 && c == ']' && bracketDepth > 0) {
bracketDepth--;
}
if (compositeDepth == 0 && fieldStarted) {
if (isLineBreak(pattern, i, logback) || c == '\n' || c == '\r') {
insertion = i;
break;
}
if (bracketDepth == 0 && (Character.isWhitespace(c) || c == '|' || c == ',' || c == ';')) {
insertion = i;
// Keep the original separator before the new fields, including delimited logs.
while (insertion < pattern.length()
&& Character.isWhitespace(pattern.charAt(insertion))
&& pattern.charAt(insertion) != '\n'
&& pattern.charAt(insertion) != '\r') {
insertion++;
}
if (insertion < pattern.length()
&& (pattern.charAt(insertion) == '|'
|| pattern.charAt(insertion) == ','
|| pattern.charAt(insertion) == ';')) {
insertion++;
while (insertion < pattern.length()
&& Character.isWhitespace(pattern.charAt(insertion))
&& pattern.charAt(insertion) != '\n'
&& pattern.charAt(insertion) != '\r') {
insertion++;
}
}
break;
}
}
fieldStarted |= !Character.isWhitespace(c);
}
StringBuilder result = new StringBuilder();
result.append(pattern, 0, insertion);
if (insertion > 0) {
char previous = pattern.charAt(insertion - 1);
if (!Character.isWhitespace(previous)
&& previous != '|'
&& previous != ','
&& previous != ';') {
result.append(' ');
}
}
if ((presentFields & SERVICE) == 0) {
result.append("dd.service=%X{dd.service} ");
}
if ((presentFields & TRACE_ID) == 0) {
result.append("dd.trace_id=%X{dd.trace_id} ");
}
if ((presentFields & SPAN_ID) == 0) {
result.append("dd.span_id=%X{dd.span_id} ");
}
if (insertion == pattern.length()
|| isLineBreak(pattern, insertion, logback)
|| pattern.charAt(insertion) == '\n'
|| pattern.charAt(insertion) == '\r') {
result.setLength(result.length() - 1);
}
return result.append(pattern, insertion, pattern.length()).toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@
0 ch.qos.logback.classic.spi.LoggingEvent*
# We instrument this Runnable
0 ch.qos.logback.core.AsyncAppenderBase$Worker
0 ch.qos.logback.core.pattern.PatternLayoutBase
2 com.beust.jcommander.*
2 com.carrotsearch.hppc.*
0 com.carrotsearch.hppc.HashOrderMixing$*
Expand Down
7 changes: 7 additions & 0 deletions dd-java-agent/instrumentation/log4j/log4j-2.0/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@ configurations {
}

muzzle {
pass {
name = 'pattern-layout'
group = 'org.apache.logging.log4j'
module = 'log4j-core'
versions = '[2.0,3.0.0)'
}

pass {
name = "default"
group = 'org.apache.logging.log4j'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package datadog.trace.instrumentation.log4j2;

import static datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers.hasClassNamed;
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.traceConfig;
import static java.util.Collections.singletonMap;
import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
import static net.bytebuddy.matcher.ElementMatchers.not;

import com.google.auto.service.AutoService;
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.agent.tooling.InstrumenterModule;
import datadog.trace.api.Config;
import java.util.HashMap;
import java.util.Map;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.matcher.ElementMatcher;

/** Provides the service field on versions predating Log4j2's ContextDataInjector. */
@AutoService(InstrumenterModule.class)
public class LogEventServiceInstrumentation extends InstrumenterModule.Tracing
implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice {
public LogEventServiceInstrumentation() {
super("log4j", "log4j-2");
}

@Override
protected boolean defaultEnabled() {
return Config.get().isLogPatternInject();
}

@Override
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return not(hasClassNamed("org.apache.logging.log4j.core.impl.ContextDataInjectorFactory"));
}

@Override
public String instrumentedType() {
return "org.apache.logging.log4j.core.impl.Log4jLogEvent";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Enrich legacy asynchronous Log4j2 events with service

On Log4j2 versions before 2.7, AsyncLogger delivers RingBufferLogEvent instances directly to appenders, bypassing the Log4jLogEvent constructors instrumented here. The legacy ThreadContext updater supplies only trace/span IDs, and the newer context-data injector is unavailable. Consequently, enabling pattern injection leaves the added service field empty despite a configured service name. Service enrichment must also cover asynchronous event population.

React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it

What each reaction means
  • 👍 Useful: correct, and you're fixing it or will.
  • 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
  • 😕 Unclear: you can't tell what it means or why it matters.
  • 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.

🤖 Bits Code Review · Open Bits AI session

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Enrich legacy async Log4j events with service

When Log4j 2.0–2.6 is configured with AsyncLogger, events are populated as RingBufferLogEvent rather than constructed as Log4jLogEvent, so this advice is never invoked (and no other instrumentation in the Log4j modules targets that event type). The pre-2.7 scope listener only places trace/span IDs in ThreadContext, so the newly inserted %X{dd.service} renders empty even with logs injection enabled; cover the async event path as well.

Useful? React with 👍 / 👎.

}

@Override
public String muzzleDirective() {
return "default";
}

@Override
public void methodAdvice(MethodTransformer transformer) {
transformer.applyAdvice(isConstructor(), getClass().getName() + "$ServiceAdvice");
}

public static class ServiceAdvice {
@Advice.OnMethodExit(suppress = Throwable.class)
public static void onExit(
@Advice.FieldValue(value = "contextMap", readOnly = false) Map<String, String> context) {
Config config = Config.get();
if (!config.isLogPatternInject()
|| !traceConfig().isLogsInjectionEnabled()
|| (context != null && context.containsKey("dd.service"))) {
return;
}
String service = config.getServiceName();
if (service == null || service.isEmpty()) {
return;
}
if (context == null || context.isEmpty()) {
context = singletonMap("dd.service", service);
} else {
Map<String, String> copy = new HashMap<>(context);
copy.put("dd.service", service);
context = copy;
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package datadog.trace.instrumentation.log4j2;

import datadog.trace.api.Config;
import datadog.trace.bootstrap.instrumentation.log.LogPatternInjector;
import net.bytebuddy.asm.Advice;
import org.apache.logging.log4j.core.layout.PatternLayout;
import org.apache.logging.log4j.core.pattern.MdcPatternConverter;

/** Rewrites the event pattern before Log4j compiles its converters. */
public class PatternLayoutConstructorAdvice {
@Advice.OnMethodEnter(suppress = Throwable.class)
public static void onEnter(@Advice.Argument(value = 2, readOnly = false) String pattern) {
if (Config.get().isLogPatternInject()) {
LogPatternInjector.Format format = LogPatternInjector.Format.LOG4J2;
try {
// Early 2.x treats a comma-separated option as a single MDC key.
MdcPatternConverter.class.getDeclaredField("keys");
} catch (NoSuchFieldException legacyVersion) {
format = LogPatternInjector.Format.LOG4J2_LEGACY;
}
pattern =
LogPatternInjector.injectLog4j(
pattern == null ? PatternLayout.DEFAULT_CONVERSION_PATTERN : pattern, format);
}
}
}
Loading
Loading