Skip to content

Commit

Permalink
Context.SetUsername takes precedence (#973)
Browse files Browse the repository at this point in the history
* Context.SetUsername takes precedence

If Context.SetUsername is called explicitly,
don't automatically set the username based on
HTTP user info (which may or may not exist.)

* Update CHANGELOG.asciidoc
  • Loading branch information
axw committed Jun 8, 2021
1 parent 239b62e commit ccd2fa7
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 8 deletions.
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

0 comments on commit ccd2fa7

Please sign in to comment.