Skip to content
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

Hooks for adding custom fields #147

Closed
echatman opened this issue Oct 7, 2021 · 9 comments · Fixed by #151
Closed

Hooks for adding custom fields #147

echatman opened this issue Oct 7, 2021 · 9 comments · Fixed by #151
Labels
agent-java community Issues and PRs created by the community

Comments

@echatman
Copy link
Contributor

echatman commented Oct 7, 2021

I would like to use co.elastic.logging.logback.EcsEncoder but I need to be able to add some custom fields. Some examples are: Exception root cause class, exception root cause message, and raw/unformatted log message.

I tried subclassing EcsEncoder, but since everything inside encode() is just static calls to EcsJsonSerializer, and all of the fields in EcsEncoder are private, I can't find any way to accomplish this.

@felixbarny
Copy link
Member

You could set custom values as MDC. Would that be an option for you?

@AlexanderWert AlexanderWert added agent-java community Issues and PRs created by the community labels Oct 11, 2021
@echatman
Copy link
Contributor Author

I'm not sure I understand the suggestion, it doesn't seem to apply for the fields I gave as examples.

Are you suggesting that I replace all log messages like this:

} catch (Exception e) {
    log.error("Initialization error", e);
}

with something like this?

} catch (Exception e) {
    try {
        Throwable rootCause = Throwables.getRootCause(e);
        MDC.put("exception_root_cause_message", rootCause.getMessage());
        MDC.put("exception_root_cause_class", rootCause.getClass().getName());
        log.error("Initialization error", e);
    } finally {
        MDC.remove("exception_root_cause_message");
        MDC.remove("exception_root_cause_class");
    }
}

I don't think that's enforceable across my entire codebase. I looked into doing it with a logback filter but I don't see a way to clear the values from the MDC after the message is logged.

@echatman
Copy link
Contributor Author

Actually I think I do see how this can help: I can subclass EcsEncoder and override encode() to add/remove fields from the MDC, something like:

    @Override
    public byte[] encode(ILoggingEvent event) {
        try {
            if (event.getThrowableProxy() != null) {
                Throwable rootCause = findRootCause(event);
                MDC.put("exception_root_cause_message", rootCause.getMessage());
                MDC.put("exception_root_cause_class", rootCause.getClass().getName());
            }
            MDC.put("unformatted_message", event.getMessage());
            return super.encode(event);
        } finally {
            MDC.remove("exception_root_cause_message");
            MDC.remove("exception_root_cause_class");
            MDC.remove("unformatted_message");
        }
    }

I think this will work, thank you!

I'd like to keep this request open, though, because I would still prefer a solution that didn't require going through the MDC.

@echatman
Copy link
Contributor Author

echatman commented Oct 12, 2021

Just finished testing out the idea in my previous comment and it doesn't work - the LoggingEvent gets it's own lazily-loaded copy of the MDC which can be populated before this point (for example: when using an AsyncAppender), so this is too late to add things to the MDC.

@felixbarny
Copy link
Member

We could add a protected method in EcsEncoder that gets called in encode, before the JSON object is closed. By default, the method does nothing but can be used by subclasses to add custom stuff to the JSON. Would that work for you?

@echatman
Copy link
Contributor Author

Yes, that'd be perfect

@felixbarny
Copy link
Member

Could you create a PR for that?

echatman added a commit to echatman/ecs-logging-java that referenced this issue Oct 18, 2021
Protected method that subclasses can override to insert arbitrary custom fields in the logback encoder. elastic#147
@felixbarny felixbarny linked a pull request Oct 19, 2021 that will close this issue
felixbarny pushed a commit that referenced this issue Oct 19, 2021
Protected method that subclasses can override to insert arbitrary custom fields in the logback encoder
closes #147
@robsonhermes
Copy link

@echatman thanks for adding this, it's perfect for my use case.

However, I extended the EcsEncoder in my spring + kotlin + logback project. When I replace the EcsEncoder with my own in logback config, the log is not produced in JSON anymore, but in the typical plain text format.

Any ideas why?

class CustomEcsEncoder : EcsEncoder() {
    override fun addCustomFields(event: ILoggingEvent?, builder: StringBuilder?) {
        requireNotNull(event) { "A logging event is required but is null" }
        val logEventAdditionalFields = convertKeyValuePairsToAdditionalFields(event.keyValuePairs)
        serializeAdditionalFields(builder, logEventAdditionalFields)
    }

    private fun convertKeyValuePairsToAdditionalFields(keyValuePairs: List<KeyValuePair>): List<AdditionalField> =
        keyValuePairs.map { AdditionalField(it.key, it.value.toString()) }
}

@robsonhermes
Copy link

Nevermind, it works now. Actually the event.keyValuePairs can be null, so, in kotlin, I need to treat it as List<KeyValuePair>?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-java community Issues and PRs created by the community
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants