[Bug] The `replaceParam` method of `AbstractLogger` behaves unexpectedly when the replaced string contains a replacement marker

[shichaoyuan]

Search before asking

• I had searched in the issues and found no similar issues.

Apache SkyWalking Component

Java Agent (apache/skywalking-java)

What happened

The replaceParam method of AbstractLogger behaves unexpectedly when the replaced string contains a replacement marker.

What you expected to happen

log.info("sample rule update: {} -> {}", "Config{a={}}", "Config{b={}}")

expect: sample rule update: Config{a={}} -> Config{b={}}

but actual: sample rule update: Config{a=Config{b={}}} -> {}

How to reproduce

just run code above

Anything else

https://github.com/apache/skywalking-java/blob/67ab8b222fbb9a71b450665eb9f733380d813b8a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/logging/core/AbstractLogger.java#L185-L203

            tmpMessage = tmpMessage.replaceFirst("\\{\\}", Matcher.quoteReplacement(String.valueOf(parameters[parametersIndex++])));
            startSize = index + 2;

The index should not just increase by 2, but should increase by the length of the string to be replaced.

Are you willing to submit a pull request to fix on your own?

• Yes I am willing to submit a pull request on my own!

Code of Conduct

• I agree to follow this project's Code of Conduct

[wu-sheng]

We don't want to use complex regex to be honest.
Why we need to put agent logs so complex? It should be just a simple version implementation, as agent plugin is the only one using it.
This is not an app logger.

[shichaoyuan]

We don't want to use complex regex to be honest. Why we need to put agent logs so complex? It should be just a simple version implementation, as agent plugin is the only one using it. This is not an app logger.

I fully agree with your point. Regular expressions are unnecessary here, simple string replacement would suffice for implementation.

    protected String replaceParam(String message, Object... parameters) {
        if (message == null) {
            return message;
        }
        int startSize = 0;
        int parametersIndex = 0;
        int index;
        StringBuilder sb = new StringBuilder(message);
        while ((index = sb.indexOf("{}", startSize)) != -1) {
            if (parametersIndex >= parameters.length) {
                break;
            }

            String tmp = String.valueOf(parameters[parametersIndex++]);
            sb.replace(index, index+2, tmp);
            startSize = index + tmp.length();
        }
        return sb.toString();
    }

[wu-sheng]

Could you explain more about which is the bug now? Or file a PR to show the code change without using regex.

[shichaoyuan]

Could you explain more about which is the bug now? Or file a PR to show the code change without using regex.

I submitted a PR apache/skywalking-java#782