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

fix: nrslog handler slog.Group #990

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

luispe
Copy link

@luispe luispe commented Dec 26, 2024

added processing for slog.Group when nested withing a log

Details

When a log.Group is added in newrelic it is replaced with a map key value that modifies the original values and empty values are stored.

e.g. of code:
https://github.com/newrelic/go-agent/tree/master/v3/integrations/logcontext-v2/nrslog/example

package main

import (
	"log/slog"
	"os"
	"time"

	"github.com/newrelic/go-agent/v3/integrations/logcontext-v2/nrslog"
	"github.com/newrelic/go-agent/v3/newrelic"
)

func main() {
	app, err := newrelic.NewApplication(
		newrelic.ConfigAppName("my-wesome-nrapp"),
		newrelic.ConfigFromEnvironment(),
		newrelic.ConfigAppLogEnabled(true),
	)
	if err != nil {
		panic(err)
	}

	app.WaitForConnection(time.Second * 5)
	log := slog.New(nrslog.TextHandler(app, os.Stdout, &slog.HandlerOptions{}))

	log.Info("I am a log message")

	log.Info("I am a log group inside a log message",
		slog.String("foo", "bar"),
		slog.Int("answer", 42),
		slog.Group("user",
			slog.String("name", "John Smith"),
			slog.String("email", "[email protected]"),
			slog.Int("age", 23),
		),
	)

	log.Info("All Done!")

	app.Shutdown(time.Second * 10)
}

Which is displayed in newrelic/logs

{
  "answer": 42,
  "entity.guid": "some_entity_guid",
  "entity.guids": "some_entity_guids",
  "entity.name": "my-wesome-nrapp",
  "foo": "bar",
  "hostname": "hostname_example",
  "level": "INFO",
  "message": "I am a log group inside a log message",
  "newrelic.logPattern": "nr.DID_NOT_MATCH",
  "newrelic.source": "logs.APM",
  "timestamp": 1735243287475,
  "user": "[{\"Key\":\"name\",\"Value\":{}},{\"Key\":\"email\",\"Value\":{}},{\"Key\":\"age\",\"Value\":{}}]"
}

@CLAassistant
Copy link

CLAassistant commented Dec 26, 2024

CLA assistant check
All committers have signed the CLA.

@luispe luispe changed the title fix: nrslog handler fix: nrslog handler slog.Group Dec 26, 2024
@iamemilio
Copy link
Contributor

Hi, thanks for contributing. If you dont mind, we are working on a handful of changes to nrslog that may impact this PR. Would you mind re-assessing this after we merge them. There are deeper rooted issues with the way groups are currently implemented in the extension that are in the process of being resolved. Once we settle that code, we will ping you and help integrate this change.

@luispe
Copy link
Author

luispe commented Jan 13, 2025

Hi, thanks for contributing. If you dont mind, we are working on a handful of changes to nrslog that may impact this PR. Would you mind re-assessing this after we merge them. There are deeper rooted issues with the way groups are currently implemented in the extension that are in the process of being resolved. Once we settle that code, we will ping you and help integrate this change.

Yes, of course @iamemilio, I will keep an eye on the ping to be able to collaborate. Keep me informed

@iamemilio
Copy link
Contributor

Ok @luispe sorry for the long wait time. I am ramping back up on the slog issues now. So, to clarify, is the issue:

A: Groups that are being passed as attributes are not being forwarded to the underlying handler. This would mean that when a group is passed as a log attribute, log prints are not getting the groups and associated attributes we are expecting.

B: Groups that are being passed as attributes are not being saved and their attributes are not being recorded by New Relic. This would mean that you are not seeing the groups and their associated attributes showing up in New Relic.

B is a known issue, and I will be working on a fix for this to address #986

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants