NR-23881 - Give users possibility to add MDC to logging #866

StanlieK · 2022-05-27T14:23:13Z

Include MDC data into logs forwarded to NR from log4j-2 and logback

Overview

This PR will add support for including MDC data in logs forwarded to NR from log4j-2 and logback. It maps the MDC map into log events prefixed by mdc.. E.g. if you have the MDC key example, it will have the key mdc.example in New Relic.

The whole feature is disabled by default and can be enabled by setting:

application_logging.forwarding.include_mdc.enabled to true
NEW_RELIC_APPLICATION_LOGGING_FORWARDING_INCLUDE_MDC_ENABLED to true

Important: This feature is not available for log4j 1.x as the agent does not support forwarding of the logs at all.

Example - Logback with disabled INCLUDE_MDC feature

Example - Logback with enabled INCLUDE_MDC feature

Example - Log4j with disabled INCLUDE_MDC feature

Example - Log4j with enabled INCLUDE_MDC feature

Related Github Issue

The feature has been requested in this issue #801.

Testing

The agent includes a suite of tests that should be used to
verify your changes don't break existing functionality. These tests will run with
Github Actions when a pull request is made. More details on running the tests locally can be found
here.

Checks

[x] Are your contributions backwards compatible with relevant frameworks and APIs?
[ ] Does your code contain any breaking changes? Please describe.
[ ] Does your code introduce any new dependencies? Please describe.

CLAassistant · 2022-05-27T14:23:19Z

All committers have signed the CLA.

StanlieK · 2022-05-31T05:10:52Z

@kford-newrelic @jasonjkeller Hey guys! May I ask for a review, please? Thanks so much

kford-newrelic · 2022-06-02T20:00:13Z

@StanlieK thanks for the PR, we really appreciate it!

We're not in a position to review this at the moment, since we're working on high-priority work for our next release but rest assured that we'll add this to our roadmap and will consider it for a future release!

fieldju · 2022-06-10T17:50:07Z

I want to plus one this PR.

I ended up implementing this slightly different on my fork.

I did a whitelist of MDC keys but I'd be perfectly happy having all the MDC keys too.

@StanlieK I'd prefer if my vars are NOT prefixed by the key mdc, so that when my go services which do not use MDC add their context variables, I can write a single query that gets the logs from the distributed microservices.

EX.

Our java services and go services add PipelineId and I want to just be able to search for the workflow id without having to do complicated queries.

On my fork I have implemented it like this

NEW_RELIC_OPTS="\
...
-Dnewrelic.config.application_logging.forward_mdc_properties.enabled=true \
-Dnewrelic.config.application_logging.forward_mdc_include_list=ActivityID,ActivityType,Attempt,RunID,TaskQueue,WorkerID,WorkflowID,WorkflowType,ExecutionID,PipelineID,Tenant,OrgID,EnvID
...
"

☝️ PipelineId can show up as a root property

fieldju · 2022-06-10T17:51:53Z

I'd also like to say that I need this PR and #849 for this to be a feature complete logging solution for my microservices.

fieldju · 2022-06-10T17:56:03Z

instrumentation/apache-log4j-2/src/main/java/com/nr/agent/instrumentation/log4j2/AgentUtil.java


                    logEventMap.put(MESSAGE, formattedMessage);
                    logEventMap.put(TIMESTAMP, event.getTimeMillis());

+                    if (isApplicationLoggingForwardingIncludeMdcEnabled() && event.getContextData() != null) {
+                        event.getContextData().forEach((key, value) -> logEventMap.put(MDC_ATTRIBUTE_PREFIX + key, value));


I'd love to make the prefix optional.

When working with distributed microservices that are multi-language / framework its nice to have a single context key to search for.

I wouldn't want to have to write queries that are WHERE mdc.foo = 'bar' AND foo = 'bar' if I could just write WHERE foo = 'bar'

I completely agree with you, I'll incorporate it.

StanlieK · 2022-06-13T11:23:21Z

I want to plus one this PR.

I ended up implementing this slightly different on my fork.

I did a whitelist of MDC keys but I'd be perfectly happy having all the MDC keys too.

@StanlieK I'd prefer if my vars are NOT prefixed by the key mdc, so that when my go services which do not use MDC add their context variables, I can write a single query that gets the logs from the distributed microservices.

EX.

Our java services and go services add PipelineId and I want to just be able to search for the workflow id without having to do complicated queries.

On my fork I have implemented it like this
NEW_RELIC_OPTS="\
...
-Dnewrelic.config.application_logging.forward_mdc_properties.enabled=true \
-Dnewrelic.config.application_logging.forward_mdc_include_list=ActivityID,ActivityType,Attempt,RunID,TaskQueue,WorkerID,WorkflowID,WorkflowType,ExecutionID,PipelineID,Tenant,OrgID,EnvID
...
"
☝️ PipelineId can show up as a root property

@fieldju I like the idea of MDC properties not being prefixed. Or at least an optional prefix.

Also, I like the idea of whitelisted/blacklisted properties. It would be nice if all the properties will be allowed by default, as we have lots of properties from project to project. It would be madness to control each separately. In this case, it would be useful to have allowed and disallowed properties. If you'll set disallowed to "*", it will exclude all the properties but allowed ones.

fieldju · 2022-06-17T05:31:53Z

@StanlieK Since they aren't too interesting in merging these PRs, we pivoted to an alternate way of shipping logs to New Relic that gives us total control.

We ended up making a custom logback layout and using the New Relic Fluent Bit helm chart

You can see/use/copy that config here in this gist

point_up this will take forward any k,v pair that is in the json blob including any MDC kv pair.

So if I add some random MDC k,v pairs like so

  @Scheduled(fixedRate = 10, timeUnit = TimeUnit.SECONDS)
  public void foo() {
    MDC.put("foo", "bar");
    MDC.put("uuid", UUID.randomUUID().toString());
    log.info("preparing to log fake errors");
    var err = new RuntimeException("fake error");
    var err2 = new RuntimeException("another fake error", err);
    log.error("fake error message", err2);
    MDC.clear();
  }

☝️ foo=bar is present in the log data in NR.

Good Luck!

kford-newrelic · 2022-07-08T01:04:03Z

@fieldju sorry that it's taking us a while to get to this PR - as you can probably imagine, it's not as simple as merging in this one PR - we need to make sure we have solutions that will work for as wide an audience as possible and that we can (hopefully) implement relatively consistently across other logging frameworks, etc.

The good new though is that we have adding MDC to our logs on our roadmap for the Jul-Sep quarter, so we expect to get to this reasonably soon!

meiao · 2022-09-09T13:45:09Z

@StanlieK writing to let you know that I started working on merging this PR.
Though for now, a lot of the work will be to get an agreement with the other language agents.

Because of that, this will not be in our upcoming 7.10 release, but most likely in our 7.11 release.

…o NR from log4j-2 and logback

Filtering mapped context attributes given include/exclude lists. Extracting common methods from AgentUtils classes to AppLogginUtil in agent-bridge

meiao · 2022-10-19T18:10:19Z

This has been tested and reviewed in #1016

mcflythekid · 2023-08-26T14:37:38Z

Please help to change this document as it is not correct

application_logging.forwarding.include_mdc.enabled to true
NEW_RELIC_APPLICATION_LOGGING_FORWARDING_INCLUDE_MDC_ENABLED to true

meiao · 2023-08-28T13:39:13Z

@mcflythekid this PR was modified before it was added to the codebase.
Please refer to the documentation for the correct configuration:
https://docs.newrelic.com/docs/apm/agents/java-agent/configuration/java-agent-configuration-config-file/#log-context-data

StanlieK force-pushed the feature/mdc-in-log-forwarding branch 2 times, most recently from fcaf826 to ec8712a Compare May 27, 2022 14:29

StanlieK mentioned this pull request May 27, 2022

Add Mapped Diagnostic Context data to LogEvents as attributes #801

Closed

StanlieK force-pushed the feature/mdc-in-log-forwarding branch 3 times, most recently from ec8712a to e2aacd6 Compare June 3, 2022 10:16

kford-newrelic added the Q2 SC label Jun 3, 2022

fieldju reviewed Jun 10, 2022

View reviewed changes

fieldju mentioned this pull request Jun 10, 2022

feature(logback-classic): add ability to include stack traces when forwarding logs #849

Closed

kford-newrelic removed the Q2 SC label Jun 14, 2022

meiao mentioned this pull request Sep 21, 2022

Internal PR for testing #866 #1016

Closed

feature/mdc-in-log-forwarding: Include MDC data into logs forwarded t…

1d07304

…o NR from log4j-2 and logback

meiao force-pushed the feature/mdc-in-log-forwarding branch from e2aacd6 to ea97eb3 Compare October 13, 2022 23:16

NR-23881 - Give users possibility to add MDC to logging

66bbdec

Filtering mapped context attributes given include/exclude lists. Extracting common methods from AgentUtils classes to AppLogginUtil in agent-bridge

meiao force-pushed the feature/mdc-in-log-forwarding branch from ea97eb3 to 66bbdec Compare October 19, 2022 18:07

meiao changed the title ~~feature/mdc-in-log-forwarding: Include MDC data into logs forwarded to NR from log4j-2 and logback~~ NR-23881 - Give users possibility to add MDC to logging Oct 19, 2022

meiao self-assigned this Oct 19, 2022

meiao self-requested a review October 19, 2022 18:10

meiao approved these changes Oct 19, 2022

View reviewed changes

meiao merged commit 8769ab2 into newrelic:main Oct 19, 2022

This was linked to issues Oct 19, 2022

Add Mapped Diagnostic Context data to LogEvents as attributes #801

Closed

An error occurred saving the clm attributes: java.lang.NullPointerException #1047

Closed

meiao removed a link to an issue Oct 24, 2022

An error occurred saving the clm attributes: java.lang.NullPointerException #1047

Closed

meiao linked an issue Oct 27, 2022 that may be closed by this pull request

[Repolinter] Open Source Policy Issues #1062

Closed

meiao removed a link to an issue Oct 27, 2022

[Repolinter] Open Source Policy Issues #1062

Closed

chris-gilbert-2 mentioned this pull request Dec 16, 2022

Java agent: Include configured labels as tags in forwarded application log events #1104

Closed

christopher-gilbert mentioned this pull request Mar 10, 2023

Feature/add tags to forwarded logs #1177

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NR-23881 - Give users possibility to add MDC to logging #866

NR-23881 - Give users possibility to add MDC to logging #866

StanlieK commented May 27, 2022 •

edited

Loading

CLAassistant commented May 27, 2022 •

edited

Loading

StanlieK commented May 31, 2022

kford-newrelic commented Jun 2, 2022

fieldju commented Jun 10, 2022 •

edited

Loading

fieldju commented Jun 10, 2022

fieldju Jun 10, 2022

StanlieK Jun 13, 2022

StanlieK commented Jun 13, 2022

fieldju commented Jun 17, 2022 •

edited

Loading

kford-newrelic commented Jul 8, 2022

meiao commented Sep 9, 2022

meiao commented Oct 19, 2022

mcflythekid commented Aug 26, 2023

meiao commented Aug 28, 2023

NR-23881 - Give users possibility to add MDC to logging #866

NR-23881 - Give users possibility to add MDC to logging #866

Conversation

StanlieK commented May 27, 2022 • edited Loading

Include MDC data into logs forwarded to NR from log4j-2 and logback

Overview

Example - Logback with disabled INCLUDE_MDC feature

Example - Logback with enabled INCLUDE_MDC feature

Example - Log4j with disabled INCLUDE_MDC feature

Example - Log4j with enabled INCLUDE_MDC feature

Related Github Issue

Testing

Checks

CLAassistant commented May 27, 2022 • edited Loading

StanlieK commented May 31, 2022

kford-newrelic commented Jun 2, 2022

fieldju commented Jun 10, 2022 • edited Loading

fieldju commented Jun 10, 2022

fieldju Jun 10, 2022

Choose a reason for hiding this comment

StanlieK Jun 13, 2022

Choose a reason for hiding this comment

StanlieK commented Jun 13, 2022

fieldju commented Jun 17, 2022 • edited Loading

kford-newrelic commented Jul 8, 2022

meiao commented Sep 9, 2022

meiao commented Oct 19, 2022

mcflythekid commented Aug 26, 2023

meiao commented Aug 28, 2023

StanlieK commented May 27, 2022 •

edited

Loading

CLAassistant commented May 27, 2022 •

edited

Loading

fieldju commented Jun 10, 2022 •

edited

Loading

fieldju commented Jun 17, 2022 •

edited

Loading