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

[chris-gilbert-2]

Description

I use the Java agent, and would like additional static information (git branch/commit hash) that I have available in environment variables to be made available as a dimension in Log events. The labels configuration allows me to easily inject that information into events other than forwarded log events, where they are visible with tags. prefix, I would like those tags to be included in logs that are forwarded by the agent.

I note that @StanlieK contributed an enhancement to include MDC in log events (#866) - this is one option but MDC isn't really the right tool here - the information I have is the same for all threads, and using MDC would mean ensuring that it gets set in every thread (http request handling, background jobs that fire etc) - that is hard to do reliably and doesn't feel right given that the value is fixed at runtime.

Another option is to use an external forwarding mechanism, and I understand that any arbitrary information can be added alongside the New Relic expected attributes, but I feel that it would be more straightforward to use the agent.

I may have missed another obvious way to achieve what I want - I am quite new to New Relic, so there may be an existing way to enhance forwarded application log events with arbitrary additional data that will be exposed as dimensions in the Log event, but I haven't managed to find it.

Acceptance Criteria

The above PR demonstrates how to look up configuration, and then add additional data to log events that come from log4j and logback forwarded data.

Intention would be to use a similar approach to add any key value pairs that are specified in the top level labels configuration to forwarded log events with a tags. prefix for consistency with other events. I'd be happy to raise a PR along those lines.

Design Consideration/Limitations

This feels like a consistent approach. I don't know if it could/should be applied to other agents, but it feels like it is in line with the previous accepted PR, as well as being aligned with the intent and usage of existing configuration.

I'm not sure if it should also be possible to switch off or explicitly enable it under the application_logging stanza to avoid any impact on existing users.

[workato-integration]

https://issues.newrelic.com/browse/NEWRELIC-5922

[christopher-gilbert]

Hi, I had a couple of days off and made some changes on a fork, just applying to Logback for the moment - the outcome is as I'd expect - labels configuration is available as metadata when looking at forwarded logs; and if I query the Log table, the structure is the same as other entities such as Transaction which pick up labels config and interpret it as tags (I can add some screenshots after checking that I have obscured work related data).

I'd appreciate any thoughts on whether this approach is okay, and whether this seems like a reasonable additional feature - it is certainly something we would find useful, and does seems to make forwarded logs more consistent with other entities - current fork changes are here - if you think this is okay I will extend to log4j and make any other tidying up/documentation/unit test changes.

The rationale for the changes I made is as follows:

I note that agent-bridge can only access config via the flattened properties rather than, say, accessing LabelsConfig and so the approach I took is to extend the existing flattening approach, which currently will parse Strings into Boolean or Integer. My changes preserve the existing behaviour, and extend it to cover List<String> and Map<String, String> using the existing parsing logic used within specific config types.

I have extracted map parsing from LabelsConfigImpl into BaseConfig so that List and Map parsing are now in the same place, and to avoid applying labels specific validation to any other arbitrary 'String as Map' data - the parsing approach could be extended to pass validation rules in, or define a static map of keys -> validators but I decided to keep things simple.

In terms of preserving existing behaviour, although the flattening doesn't apply any validation, the config is still also parsed into LabelsConfigImpl and so labels are still validated (and all existing unit tests pass). The only behaviour change I have made is to trim the keys and values BEFORE the length is validated - previously trimming whitespace happened afterwards - I think it is more correct to validate the value that is actually sent by the agent - at the moment we might see a warning that labels have been truncated to 255 characters but the value could be shorter if the 255 included leading/trailing whitespace. I can amend this though if needed to avoid any change.

Another point to note is that only the tags that are defined in labels config are added to forwarded Log metadata, none of the tags that are added by the agent (eg instrumentation.name, instrumentation.provider etc) are included so it means forwarded logs aren't quite consistent with other entities, but looking at the code I couldn't see those tags being applied within the agent so I'm wondering if they are added at the server end? The config tags are fine for our use case but if there is a need to garnish forwarded logs with other tags, I'd need a steer on where that information is added to other entities.

[chris-gilbert-2]

A couple of screenshots after building from my forked branch, and configuring an AWS task definition with the following:

          Environment:
            - Name: NEW_RELIC_APP_NAME
              Value: !Sub "${AWS::StackName}"
            - Name: NEW_RELIC_LABELS
              Value: "cg-test1:test-value1;cg-test2:test-value2"

---

1. Querying the Log table shows the new attributes that were added

---

2. The way they are visible in Log is consistent with other entities that already incorporate any configured labels eg Transaction

---

3. Searching logs by tag is consistent with searching any other entity that currently incorporates configured labels:

---

---

4. Configured labels are visible in log metadata

---

5. Configured labels are already shown as tags on applications, but alongside other tags - these other tags are not shown in log metadata

[kford-newrelic]

Hi @chris-gilbert-2 do you want to submit a PR for your idea? If yes, we can certainly review your idea in that context

If not, we will certainly consider your suggested enhancement

[chris-gilbert-2]

Great - I have an implementation on a fork. I built and ran it locally, but since then I have merged in the latest changes from main branch. I'll merge in any other new changes and I'll rebuild and check it all works then raise a PR.

[christopher-gilbert]

Hi - PR raised at #1177 - I've added a couple of explanatory comments.

[kford-newrelic]

@chris-gilbert-2 our engineers thought your suggestion was a good one - but they felt that it should be considered for standardization in our agent specification (a good thing; and subsequently implemented across all our APM agents). We're going to trigger those discussions during this Apr-Jun quarter but because that takes time to get buy in from other engineers, we'll have to keep your PR on hold for the moment.

[chris-gilbert-2]

That's great news - thanks, give me a shout any time if you need to check anything with me

[jasonjkeller]

@chris-gilbert-2 We've done some investigation into this request and it appears to be the case that all other event types (e.g. Span Events, Transaction Events, Custom Events, Error Events) that currently have labels automatically added to them, have such decoration done via a backend pipeline.

We think the right approach is to keep this consistent and also have Log Events be enhanced with label attributes via the same backend pipeline, rather than being an outlier and doing it via the agent, while also adding a mechanism to make the feature user configurable. To that end, we're going to close out this request and let our logging platform team prioritize such a solution on their roadmap.

Thanks again for bringing this request to us and prototyping a potential solution! Much appreciated!