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

Context.SetUsername takes precedence #973

Merged
merged 3 commits into from Jun 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.asciidoc
Expand Up @@ -24,6 +24,7 @@ endif::[]
https://github.com/elastic/apm-agent-go/compare/v1.12.0...master[View commits]

- Prefer w3c traceparent header over legacy elastic-apm-traceparent {pull}963[#(963)]
- Context.SetUsername now takes precedence over HTTP user info from Context.SetHTTPRequest {pull}973[#(973)]

[[release-notes-1.x]]
=== Go Agent version 1.x
Expand Down
19 changes: 11 additions & 8 deletions context.go
Expand Up @@ -159,7 +159,8 @@ func (c *Context) SetFramework(name, version string) {
// If the request contains HTTP Basic Authentication, the username
// from that will be recorded in the context. Otherwise, if the
// request contains user info in the URL (i.e. a client-side URL),
// that will be used.
// that will be used. An explicit call to SetUsername always takes
// precedence.
func (c *Context) SetHTTPRequest(req *http.Request) {
// Special cases to avoid calling into fmt.Sprintf in most cases.
var httpVersion string
Expand Down Expand Up @@ -203,13 +204,15 @@ func (c *Context) SetHTTPRequest(req *http.Request) {
c.request.Socket = &c.requestSocket
}

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
if c.model.User == nil {
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
}
}
}

Expand Down
14 changes: 14 additions & 0 deletions context_test.go
Expand Up @@ -19,6 +19,8 @@ package apm_test

import (
"context"
"net/http"
"net/url"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -119,6 +121,18 @@ func TestContextCustom(t *testing.T) {
}, tx.Context.Custom)
}

func TestContextSetUsernamePrecedence(t *testing.T) {
tx := testSendTransaction(t, func(tx *apm.Transaction) {
tx.Context.SetUsername("frieda")
req, _ := http.NewRequest("GET", "/", nil)
req.URL.User = url.User("fred")
tx.Context.SetHTTPRequest(req)
})
require.NotNil(t, tx.Context)
require.NotNil(t, "", tx.Context.User)
assert.Equal(t, "frieda", tx.Context.User.Username)
}

func testSendTransaction(t *testing.T, f func(tx *apm.Transaction)) model.Transaction {
transaction, _, _ := apmtest.WithTransaction(func(ctx context.Context) {
f(apm.TransactionFromContext(ctx))
Expand Down