Revisited APM spans/transactions set (#1306)

This PR reviews the APM spans that are set in Elastic Package Registry.
It add news spans to allow us to have a better knowledge of the
requests, specially for the proxied requests.

Author: mrodm

CHANGELOG.md

@@ -13,10 +13,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * Update Go runtime to 1.24.2. [#1299](https://github.com/elastic/package-registry/pull/1299)
 * Ignore unknown categories instead of producing fatal errors. [#1297](https://github.com/elastic/package-registry/pull/1297)
 * Fix usages of time.Since in defer statements used to obtain duration Prometheus metrics in the indexer. [#1304](https://github.com/elastic/package-registry/pull/1304)
+* Rename some spans to avoid conflicts. [#1306](https://github.com/elastic/package-registry/pull/1306)
 
 ### Added
 
 * Change license from Elastic License to Elastic License 2.0. [#1298](https://github.com/elastic/package-registry/pull/1298)
+* Add APM spans for proxy requests and storage indexer Get calls. [#1306](https://github.com/elastic/package-registry/pull/1306)
 
 ### Deprecated
 

packages/http.go

@@ -63,7 +63,8 @@ func ServePackageSignature(logger *zap.Logger, w http.ResponseWriter, r *http.Re
 }
 
 func serveLocalPackage(logger *zap.Logger, w http.ResponseWriter, r *http.Request, p *Package, packagePath string) {
-	span, _ := apm.StartSpan(r.Context(), "ServePackage", "app")
+	span, _ := apm.StartSpan(r.Context(), "ServeLocalPackage", "app")
+	span.Context.SetLabel("file.name", packagePath)
 	defer span.End()
 
 	logger = logger.With(zap.String("file.name", packagePath))
@@ -93,7 +94,8 @@ func serveLocalPackage(logger *zap.Logger, w http.ResponseWriter, r *http.Reques
 
 // ServePackageResource is used by staticHandler.
 func ServePackageResource(logger *zap.Logger, w http.ResponseWriter, r *http.Request, p *Package, packageFilePath string) {
-	span, _ := apm.StartSpan(r.Context(), "ServePackage", "app")
+	span, _ := apm.StartSpan(r.Context(), "ServePackageResource", "app")
+	span.Context.SetLabel("file.name", packageFilePath)
 	defer span.End()
 
 	if p.RemoteResolver() != nil {

packages/packages.go

@@ -198,6 +198,10 @@ func (i *FileSystemIndexer) Get(ctx context.Context, opts *GetOptions) (Packages
 	defer func() {
 		metrics.IndexerGetDurationSeconds.With(prometheus.Labels{"indexer": i.label}).Observe(time.Since(start).Seconds())
 	}()
+	span, ctx := apm.StartSpan(ctx, "GetFileSystemIndexer", "app")
+	span.Context.SetLabel("indexer", i.label)
+	defer span.End()
+
 	if opts == nil {
 		return i.packageList, nil
 	}

proxymode/proxymode.go

@@ -16,6 +16,7 @@ import (
 
 	"github.com/gorilla/mux"
 	"github.com/hashicorp/go-retryablehttp"
+	"go.elastic.co/apm/v2"
 
 	"go.uber.org/zap"
 
@@ -112,6 +113,9 @@ func (pm *ProxyMode) Enabled() bool {
 }
 
 func (pm *ProxyMode) Search(r *http.Request) (packages.Packages, error) {
+	span, _ := apm.StartSpan(r.Context(), "Proxy Search", "app")
+	defer span.End()
+
 	proxyURL := *r.URL
 	proxyURL.Host = pm.destinationURL.Host
 	proxyURL.Scheme = pm.destinationURL.Scheme
@@ -140,6 +144,9 @@ func (pm *ProxyMode) Search(r *http.Request) (packages.Packages, error) {
 }
 
 func (pm *ProxyMode) Categories(r *http.Request) ([]packages.Category, error) {
+	span, _ := apm.StartSpan(r.Context(), "Proxy Categories", "app")
+	defer span.End()
+
 	proxyURL := *r.URL
 	proxyURL.Host = pm.destinationURL.Host
 	proxyURL.Scheme = pm.destinationURL.Scheme
@@ -165,6 +172,9 @@ func (pm *ProxyMode) Categories(r *http.Request) ([]packages.Category, error) {
 }
 
 func (pm *ProxyMode) Package(r *http.Request) (*packages.Package, error) {
+	span, _ := apm.StartSpan(r.Context(), "Proxy Package", "app")
+	defer span.End()
+
 	vars := mux.Vars(r)
 	packageName, ok := vars["packageName"]
 	if !ok {

storage/indexer.go

@@ -208,6 +208,9 @@ func (i *Indexer) Get(ctx context.Context, opts *packages.GetOptions) (packages.
 		metrics.IndexerGetDurationSeconds.With(prometheus.Labels{"indexer": indexerGetDurationPrometheusLabel}).Observe(time.Since(start).Seconds())
 	}()
 
+	span, ctx := apm.StartSpan(ctx, "GetStorageIndexer", "app")
+	defer span.End()
+
 	i.m.RLock()
 	defer i.m.RUnlock()
 

storage/indexer_test.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap/zapcore"
 
@@ -86,70 +87,148 @@ func BenchmarkIndexerGet(b *testing.B) {
 	})
 }
 
-func TestGet_ListAllPackages(t *testing.T) {
+func TestGet_ListPackages(t *testing.T) {
 	// given
 	fs := PrepareFakeServer(t, "testdata/search-index-all-full.json")
 	defer fs.Stop()
 	storageClient := fs.Client()
 	indexer := NewIndexer(util.NewTestLogger(), storageClient, FakeIndexerOptions)
 
-	err := indexer.Init(context.Background())
+	ctx := context.Background()
+	err := indexer.Init(ctx)
 	require.NoError(t, err, "storage indexer must be initialized properly")
 
-	// when
-	foundPackages, err := indexer.Get(context.Background(), &packages.GetOptions{})
-
-	// then
-	require.NoError(t, err, "packages should be returned")
-	require.Len(t, foundPackages, 1133)
-}
-
-func TestGet_FindLatestPackage(t *testing.T) {
-	// given
-	fs := PrepareFakeServer(t, "testdata/search-index-all-full.json")
-	defer fs.Stop()
-	storageClient := fs.Client()
-	indexer := NewIndexer(util.NewTestLogger(), storageClient, FakeIndexerOptions)
-
-	err := indexer.Init(context.Background())
-	require.NoError(t, err, "storage indexer must be initialized properly")
-
-	// when
-	foundPackages, err := indexer.Get(context.Background(), &packages.GetOptions{
-		Filter: &packages.Filter{
-			PackageName: "apm",
-			PackageType: "integration",
+	cases := []struct {
+		name            string
+		options         *packages.GetOptions
+		expected        int
+		expectedName    string
+		expectedVersion string
+	}{
+		{
+			name:     "all packages filter nil",
+			options:  &packages.GetOptions{},
+			expected: 1133,
 		},
-	})
-
-	// then
-	require.NoError(t, err, "packages should be returned")
-	require.Len(t, foundPackages, 1)
-	require.Equal(t, "apm", foundPackages[0].Name)
-	require.Equal(t, "8.2.0", foundPackages[0].Version)
-}
-
-func TestGet_UnknownPackage(t *testing.T) {
-	// given
-	fs := PrepareFakeServer(t, "testdata/search-index-all-full.json")
-	defer fs.Stop()
-	storageClient := fs.Client()
-	indexer := NewIndexer(util.NewTestLogger(), storageClient, FakeIndexerOptions)
-
-	err := indexer.Init(context.Background())
-	require.NoError(t, err, "storage indexer must be initialized properly")
-
-	// when
-	foundPackages, err := indexer.Get(context.Background(), &packages.GetOptions{
-		Filter: &packages.Filter{
-			PackageName: "qwertyuiop",
-			PackageType: "integration",
+		{
+			name: "all versions of packages including prerelease",
+			options: &packages.GetOptions{
+				Filter: &packages.Filter{
+					AllVersions: true,
+					Prerelease:  true,
+				},
+			},
+			expected: 1133,
 		},
-	})
+		{
+			name: "latest versions of packages not including prerelease",
+			options: &packages.GetOptions{
+				Filter: &packages.Filter{
+					AllVersions: false,
+					Prerelease:  false,
+				},
+			},
+			expected: 99,
+		},
+		{
+			name: "all packages with all versions and no prerelease",
+			options: &packages.GetOptions{
+				Filter: &packages.Filter{
+					AllVersions: true,
+				},
+			},
+			expected: 494,
+		},
+		{
+			name: "all packages with latest versions and no prerelease",
+			options: &packages.GetOptions{
+				Filter: &packages.Filter{
+					Prerelease: false,
+				},
+			},
+			expected: 99,
+		},
+		{
+			name: "all packages prerelease",
+			options: &packages.GetOptions{
+				Filter: &packages.Filter{
+					Prerelease: true,
+				},
+			},
+			expected: 147,
+		},
+		{
+			name: "all zeek packages with prerelease",
+			options: &packages.GetOptions{
+				Filter: &packages.Filter{
+					AllVersions: true,
+					Prerelease:  true,
+					PackageName: "zeek",
+				},
+			},
+			expected: 17,
+		},
+		{
+			name: "all packages with all versions of a giventype",
+			options: &packages.GetOptions{
+				Filter: &packages.Filter{
+					AllVersions: true,
+					Prerelease:  true,
+					PackageType: "solution",
+				},
+			},
+			expected: 2,
+		},
+		{
+			name: "one package of a giventype",
+			options: &packages.GetOptions{
+				Filter: &packages.Filter{
+					Prerelease:     true,
+					PackageName:    "tomcat",
+					PackageVersion: "0.3.0",
+				},
+			},
+			expected: 1,
+		},
+		{
+			name: "unknown package",
+			options: &packages.GetOptions{
+				Filter: &packages.Filter{
+					PackageName: "qwertyuiop",
+					PackageType: "integration",
+				},
+			},
+			expected: 0,
+		},
+		{
+			name: "latest package",
+			options: &packages.GetOptions{
+				Filter: &packages.Filter{
+					PackageName: "apm",
+					PackageType: "integration",
+				},
+			},
+			expected:        1,
+			expectedName:    "apm",
+			expectedVersion: "8.2.0",
+		},
+	}
 
-	// then
-	require.NoError(t, err, "packages should be returned")
-	require.Len(t, foundPackages, 0)
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			// when
+			foundPackages, err := indexer.Get(ctx, c.options)
+			// then
+			require.NoError(t, err, "packages should be returned")
+			require.Len(t, foundPackages, c.expected)
+			if c.expectedName != "" {
+				assert.Equal(t, c.expectedName, foundPackages[0].Name)
+			}
+			if c.expectedVersion != "" {
+				assert.Equal(t, c.expectedVersion, foundPackages[0].Version)
+			}
+		})
+	}
 }
 
 func TestGet_IndexUpdated(t *testing.T) {