internal: cache build info to reduce lock contention in GetGitMetadat…

…aTags (#2770)

Co-authored-by: Eliott Bouhana <[email protected]>

ddtrace/tracer/tracer_test.go

@@ -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)
@@ -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:[email protected]/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)
@@ -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)
@@ -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:[email protected]/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)
@@ -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)
@@ -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)

internal/gitmetadata.go

@@ -40,8 +40,7 @@ const (
 )
 
 var (
-	lock = sync.Mutex{}
-
+	initOnce        sync.Once
 	gitMetadataTags map[string]string
 )
 
@@ -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
@@ -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

internal/gitmetadata_test.go

@@ -6,6 +6,7 @@
 package internal
 
 import (
+	"runtime/debug"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -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()
+	}
+}

profiler/upload_test.go

@@ -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)
@@ -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://[email protected]/user/repo go_path:somepath")
+		maininternal.RefreshGitMetadataTags()
+
 		profile := doOneShortProfileUpload(t)
 
 		assert := assert.New(t)
@@ -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://[email protected]/user/repo_new")
 		t.Setenv(maininternal.EnvGitCommitSha, "123456789ABCDE")
+		maininternal.RefreshGitMetadataTags()
+
 		profile := doOneShortProfileUpload(t)
 
 		assert := assert.New(t)
@@ -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)