nrzap log in context ignores additional fields

[NicholasGWK]

Description

I'm unsure if this is intended behaviour or not, but while debugging I noticed the nrzap logs in context integration does NOT do anything with zap fields...my initial expectation was that these would be added as custom attributes to the log entries.

The code here shows that the fields are just not used:

go-agent/v3/integrations/logcontext-v2/nrzap/nrzap.go

Line 28 in 046b4fc

func (nr *newrelicApplicationState) recordLog(entry zapcore.Entry, fields []zap.Field) {

Please let me know if this intended or if there's another way to add these fields as attributes!

Steps to Reproduce

• Setup nrzap integration
• Create a transaction logger similar to the example:

  go-agent/v3/integrations/logcontext-v2/nrzap/example/main.go

  Line 35 in 046b4fc

  txnCore, err := nrzap.WrapTransactionCore(core, txn)

• Log a message with additional zap fields, i.e. instead of:

txnLogger.Error("Some error!")

do

txnLogger.Error("Some error!", zap.Int("http.statusCode", http.StatusInternalServerError)

Expected Behavior

The log entry in New Relic would have a custom attributes, "http.statusCode" with the passed value.

I didn't include env info because I'm not sure it matters based on the code I linked to, but please let me know if any additional info is needed!

Thanks so much,

Nick

[stefanoschrs]

Do we have any updates on this?

Adding some fields to both logs in the example doesn't work for me either as intended:

	backgroundLogger.Info("this is a background log message", zap.String("foo", "bar"))
...
	txnLogger.Info("this is a transaction log message", zap.String("foo", "bar"))

Gives this in the console:

{"level":"info","ts":1695784536.173117,"msg":"this is a background log message","foo":"bar"}
{"level":"info","ts":1695784536.173432,"msg":"this is a transaction log message","foo":"bar"}

but in New Relic:

{
  "entity.guid": "xxx",
  "entity.guids": "xxx",
  "entity.name": "xxx",
  "hostname": "xxx",
  "level": "info",
  "message": "this is a background log message",
  "newrelic.source": "xxx",
  "timestamp": xxx
}

{
  "entity.guid": "xxx",
  "entity.guids": "xxx",
  "entity.name": "xxx",
  "hostname": "xxx",
  "level": "info",
  "message": "this is a transaction log message",
  "newrelic.source": "xxx",
  "span.id": "xxx",
  "timestamp": xxx,
  "trace.id": "xxx"
}

[iamemilio]

Yeah, we are aware of this. There is a roadmap item to add support for logs in context fields in the future, and a possible re-write of the zap libraries that will pair with it. The collection of fields as attributes is not supported in the agent yet.

[stefanoschrs]

Thanks for the reply, is there a very rough estimation on when will this happen? Just to know if it's worth waiting or switching to the REST agent way.

[adomaskizogian]

bumping this
just run into the same issue with slog.

it is sad that such core functionality is missing and forces us to do parsing of logs on ui

@iamemilio
I don't believe this should be labeled as an enhancement. It is rather a bug. It is a core functionality that simply does not work. Either this or the logcontext-v2 lib MVP scope was poorly defined and it should not had been made public. I can't imagine anyone making a decision to not support attribute capture and call it a properly implemented library in 2024 which is authored by observability platform engineers. If anything, new relic should be driving the observability practices and application to next level, not to bring it down to unstructured logging.

NewRelic go-agent only recently started supporting more than logrus and yet it fails to do it right. Despite that the only option remains to use the plugin coming from logcontext v1 with logrus cause it supports attribute capture. So why even bother implementing the new integration labraries if none of them support core must have functionality. This forces customers to parse unstructured log data using regexp and nrql to make anything useful with dashboards/alerts/insights and/or locks in with logrus which is in maintenance mode only.

[iamemilio]

Hi all, thank you for your advocacy and your patience. I hear your concerns and have gotten the go ahead to prioritize this ASAP. Supporting attribute collection for all v2 logs in context packages is my top priority. I will communicate expectations and upcoming changes here and in #833.

[iamemilio]

I am working on adding support for attribute collection to the agent now. Once I have that code settled and tested, I will prioritize Zap and then Slog for attribute collection support first. Zerolog's opacity will make attribute collection more difficult, so I'll save it for last.

[iamemilio]

Entering the review phase for attribute collection support in the agent: #900. Building this into wrappers for logging frameworks and additional optimizations are on their way. This should make the next release. Feel free to leave feedback on this PR until then.

Notes:

• capture of structs, maps, slices, and arrays are included and will be encoded as a JSON string using encoding/json.Marshal(). The backend will parse and decode these strings. If the JSON string exceeds 256 bytes, it will be truncated and will show up as just a truncated string.
• passing complex objects is probably faster if they are encoded to a string in the logging framework since those tools have much lower allocation costs for constructing JSON strings than the standard library encoder does.

[mirackara]

#902 is the WIP PR specifically for zap attribute collection. We are currently in the testing phase for attribute collection for zap. Along with #900, this should also be in the next release. Feel free to leave feedback/suggestions on this PR as well!

[iamemilio]

This feature has been released for Zap in the 3.33.0 version of the agent! #908