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

apm.Context.SetHTTPRequest wipes c.user.Username set manually #970

Closed
mcgrawia opened this issue Jun 4, 2021 · 2 comments · Fixed by #973
Closed

apm.Context.SetHTTPRequest wipes c.user.Username set manually #970

mcgrawia opened this issue Jun 4, 2021 · 2 comments · Fixed by #973
Labels

Comments

@mcgrawia
Copy link

mcgrawia commented Jun 4, 2021

Describe the bug
In my application, I am manually calling apm.Context.SetUsername to add user info to the transaction. When the transaction is set to the APM server, the username is wiped from the data in SetHTTPRequest. I've traced the problem to the following lines, which don't take into account when c.user.Username is already set:
https://github.com/elastic/apm-agent-go/blob/master/context.go#L206-L213

	username, _, ok := req.BasicAuth()
	if !ok && req.URL.User != nil {
		username = req.URL.User.Username()
	}
	c.user.Username = truncateString(username)
	if c.user.Username != "" {
		c.model.User = &c.user
	}

It seems like this line: c.user.Username = truncateString(username) sets both c.user.Username and c.model.User.Username since they point to the same model.User object.

To Reproduce

  1. In your http application, call tx.Context.SetUsername(username)
  2. observe no username is present in the APM events

Expected behavior
Username should be set in the APM events.

@axw
Copy link
Member

axw commented Jun 8, 2021

Thanks for the report @mcgrawia! I'll get onto a fix shortly.

@mcgrawia
Copy link
Author

mcgrawia commented Jun 8, 2021

thanks for the quick fix @axw! the change works great.

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

Successfully merging a pull request may close this issue.

2 participants