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.

Sign up for GitHub

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

Jump to bottom

internal: cache build info to reduce lock contention in GetGitMetadataTags #2770

Merged
merged 4 commits into from
Jul 12, 2024
Merged

internal: cache build info to reduce lock contention in GetGitMetadataTags #2770

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f4ac899
internal: cache build info to reduce lock contention on initialization
darccio Jul 12, 2024
c3c01f3
internal: fix RefreshGitMetadataTags comment
darccio Jul 12, 2024
3d10986
internal: apply suggestions from code review
darccio Jul 12, 2024
fcb1ad4
Merge branch 'main' into dario.castane/reduce-lock-contention-getgitm…
darccio Jul 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions ddtrace/tracer/tracer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1862,14 +1862,12 @@ func TestEnvironment(t *testing.T) {
}

func TestGitMetadata(t *testing.T) {
maininternal.ResetGitMetadataTags()

t.Run("git-metadata-from-dd-tags", func(t *testing.T) {
t.Setenv(maininternal.EnvDDTags, "git.commit.sha:123456789ABCD git.repository_url:github.com/user/repo go_path:somepath")
maininternal.RefreshGitMetadataTags()

tracer, _, _, stop := startTestTracer(t)
defer stop()
defer maininternal.ResetGitMetadataTags()

assert := assert.New(t)
sp := tracer.StartSpan("http.request").(*span)
Expand All @@ -1882,10 +1880,10 @@ func TestGitMetadata(t *testing.T) {

t.Run("git-metadata-from-dd-tags-with-credentials", func(t *testing.T) {
t.Setenv(maininternal.EnvDDTags, "git.commit.sha:123456789ABCD git.repository_url:https://user:passwd@github.com/user/repo go_path:somepath")
maininternal.RefreshGitMetadataTags()

tracer, _, _, stop := startTestTracer(t)
defer stop()
defer maininternal.ResetGitMetadataTags()

assert := assert.New(t)
sp := tracer.StartSpan("http.request").(*span)
Expand All @@ -1902,10 +1900,10 @@ func TestGitMetadata(t *testing.T) {
// git metadata env has priority over DD_TAGS
t.Setenv(maininternal.EnvGitRepositoryURL, "github.com/user/repo_new")
t.Setenv(maininternal.EnvGitCommitSha, "123456789ABCDE")
maininternal.RefreshGitMetadataTags()

tracer, _, _, stop := startTestTracer(t)
defer stop()
defer maininternal.ResetGitMetadataTags()

assert := assert.New(t)
sp := tracer.StartSpan("http.request").(*span)
Expand All @@ -1918,10 +1916,10 @@ func TestGitMetadata(t *testing.T) {
t.Run("git-metadata-from-env-with-credentials", func(t *testing.T) {
t.Setenv(maininternal.EnvGitRepositoryURL, "https://u:t@github.com/user/repo_new")
t.Setenv(maininternal.EnvGitCommitSha, "123456789ABCDE")
maininternal.RefreshGitMetadataTags()

tracer, _, _, stop := startTestTracer(t)
defer stop()
defer maininternal.ResetGitMetadataTags()

assert := assert.New(t)
sp := tracer.StartSpan("http.request").(*span)
Expand All @@ -1934,10 +1932,10 @@ func TestGitMetadata(t *testing.T) {
t.Run("git-metadata-from-env-and-tags", func(t *testing.T) {
t.Setenv(maininternal.EnvDDTags, "git.commit.sha:123456789ABCD")
t.Setenv(maininternal.EnvGitRepositoryURL, "github.com/user/repo")
maininternal.RefreshGitMetadataTags()

tracer, _, _, stop := startTestTracer(t)
defer stop()
defer maininternal.ResetGitMetadataTags()

assert := assert.New(t)
sp := tracer.StartSpan("http.request").(*span)
Expand All @@ -1953,10 +1951,10 @@ func TestGitMetadata(t *testing.T) {
t.Setenv(maininternal.EnvDDTags, "git.commit.sha:123456789ABCD git.repository_url:github.com/user/repo")
t.Setenv(maininternal.EnvGitRepositoryURL, "github.com/user/repo_new")
t.Setenv(maininternal.EnvGitCommitSha, "123456789ABCDE")
maininternal.RefreshGitMetadataTags()

tracer, _, _, stop := startTestTracer(t)
defer stop()
defer maininternal.ResetGitMetadataTags()

assert := assert.New(t)
sp := tracer.StartSpan("http.request").(*span)
Expand Down
34 changes: 13 additions & 21 deletions internal/gitmetadata.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ const (
)

var (
lock = sync.Mutex{}

initOnce sync.Once
gitMetadataTags map[string]string
)

Expand Down Expand Up @@ -76,10 +75,10 @@ func getTagsFromDDTags() map[string]string {
}
}

// getTagsFromBinary extracts git metadata from binary metadata
func getTagsFromBinary() map[string]string {
// getTagsFromBinary extracts git metadata from binary metadata.
func getTagsFromBinary(readBuildInfo func() (*debug.BuildInfo, bool)) map[string]string {
res := make(map[string]string)
info, ok := debug.ReadBuildInfo()
info, ok := readBuildInfo()
if !ok {
log.Debug("ReadBuildInfo failed, skip source code metadata extracting")
return res
Expand All @@ -102,32 +101,25 @@ func getTagsFromBinary() map[string]string {
return res
}

// GetGitMetadataTags returns git metadata tags
// GetGitMetadataTags returns git metadata tags. Returned map is read-only
func GetGitMetadataTags() map[string]string {
lock.Lock()
defer lock.Unlock()

if gitMetadataTags != nil {
return gitMetadataTags
}
initOnce.Do(initGitMetadataTags)
return gitMetadataTags
}

func initGitMetadataTags() {
gitMetadataTags = make(map[string]string)

if BoolEnv(EnvGitMetadataEnabledFlag, true) {
updateAllTags(gitMetadataTags, getTagsFromEnv())
updateAllTags(gitMetadataTags, getTagsFromDDTags())
updateAllTags(gitMetadataTags, getTagsFromBinary())
updateAllTags(gitMetadataTags, getTagsFromBinary(debug.ReadBuildInfo))
}

return gitMetadataTags
}

// ResetGitMetadataTags reset cashed metadata tags
func ResetGitMetadataTags() {
lock.Lock()
defer lock.Unlock()

gitMetadataTags = nil
// RefreshGitMetadataTags reset cached metadata tags. NOT thread-safe, use for testing only
func RefreshGitMetadataTags() {
initGitMetadataTags()
}

// CleanGitMetadataTags cleans up tags from git metadata
Expand Down
60 changes: 60 additions & 0 deletions internal/gitmetadata_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package internal

import (
"runtime/debug"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -54,3 +55,62 @@ func TestRemoveCredentials(t *testing.T) {
})
}
}

func TestGetTagsFromBinary(t *testing.T) {
testCases := []struct {
name string
in string
expected map[string]string
}{
{
name: "empty build info",
expected: map[string]string{},
},
{
name: "build info with module path",
expected: map[string]string{
TagGoPath: "github.com/DataDog/dd-trace-go",
},
},
{
name: "build info with module path and git repository",
expected: map[string]string{
TagGoPath: "github.com/DataDog/dd-trace-go",
TagCommitSha: "123456",
},
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
readBuildInfo := func() (*debug.BuildInfo, bool) {
info := &debug.BuildInfo{
Settings: []debug.BuildSetting{
{
Key: "vcs",
Value: "git",
},
},
}
if tc.expected[TagGoPath] != "" {
info.Path = tc.expected[TagGoPath]
}
if tc.expected[TagCommitSha] != "" {
info.Settings = append(info.Settings, debug.BuildSetting{
Key: "vcs.revision",
Value: tc.expected[TagCommitSha],
})
}
return info, true
}
tags := getTagsFromBinary(readBuildInfo)
assert.Subset(t, tags, tc.expected)
})
}
}

func BenchmarkGetGitMetadataTags(b *testing.B) {
b.Setenv(EnvGitMetadataEnabledFlag, "true")
for i := 0; i < b.N; i++ {
GetGitMetadataTags()
}
}
18 changes: 10 additions & 8 deletions profiler/upload_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,10 @@ func TestEntityContainerIDHeaders(t *testing.T) {
}

func TestGitMetadata(t *testing.T) {
maininternal.ResetGitMetadataTags()
defer maininternal.ResetGitMetadataTags()

t.Run("git-metadata-from-dd-tags", func(t *testing.T) {
maininternal.ResetGitMetadataTags()
t.Setenv(maininternal.EnvDDTags, "git.commit.sha:123456789ABCD git.repository_url:github.com/user/repo go_path:somepath")
maininternal.RefreshGitMetadataTags()

profile := doOneShortProfileUpload(t)

assert := assert.New(t)
Expand All @@ -134,8 +132,9 @@ func TestGitMetadata(t *testing.T) {
assert.Contains(profile.tags, "go_path:somepath")
})
t.Run("git-metadata-from-dd-tags-with-credentials", func(t *testing.T) {
maininternal.ResetGitMetadataTags()
t.Setenv(maininternal.EnvDDTags, "git.commit.sha:123456789ABCD git.repository_url:http://u@github.com/user/repo go_path:somepath")
maininternal.RefreshGitMetadataTags()

profile := doOneShortProfileUpload(t)

assert := assert.New(t)
Expand All @@ -144,22 +143,24 @@ func TestGitMetadata(t *testing.T) {
assert.Contains(profile.tags, "go_path:somepath")
})
t.Run("git-metadata-from-env", func(t *testing.T) {
maininternal.ResetGitMetadataTags()
t.Setenv(maininternal.EnvDDTags, "git.commit.sha:123456789ABCD git.repository_url:github.com/user/repo")

// git metadata env has priority under DD_TAGS
t.Setenv(maininternal.EnvGitRepositoryURL, "github.com/user/repo_new")
t.Setenv(maininternal.EnvGitCommitSha, "123456789ABCDE")
maininternal.RefreshGitMetadataTags()

profile := doOneShortProfileUpload(t)

assert := assert.New(t)
assert.Contains(profile.tags, "git.commit.sha:123456789ABCDE")
assert.Contains(profile.tags, "git.repository_url:github.com/user/repo_new")
})
t.Run("git-metadata-from-env-with-credentials", func(t *testing.T) {
maininternal.ResetGitMetadataTags()
t.Setenv(maininternal.EnvGitRepositoryURL, "https://u@github.com/user/repo_new")
t.Setenv(maininternal.EnvGitCommitSha, "123456789ABCDE")
maininternal.RefreshGitMetadataTags()

profile := doOneShortProfileUpload(t)

assert := assert.New(t)
Expand All @@ -168,11 +169,12 @@ func TestGitMetadata(t *testing.T) {
})

t.Run("git-metadata-disabled", func(t *testing.T) {
maininternal.ResetGitMetadataTags()
t.Setenv(maininternal.EnvGitMetadataEnabledFlag, "false")
t.Setenv(maininternal.EnvDDTags, "git.commit.sha:123456789ABCD git.repository_url:github.com/user/repo")
t.Setenv(maininternal.EnvGitRepositoryURL, "github.com/user/repo")
t.Setenv(maininternal.EnvGitCommitSha, "123456789ABCD")
maininternal.RefreshGitMetadataTags()

profile := doOneShortProfileUpload(t)

assert := assert.New(t)
Expand Down
Loading