From 545712874af113cd94c63caec9a7cfc546171c7a Mon Sep 17 00:00:00 2001 From: kruskal <99559985+kruskall@users.noreply.github.com> Date: Mon, 27 Feb 2023 02:58:41 +0100 Subject: [PATCH 01/13] test: disable kibana fallback to avoid false positives --- systemtest/sourcemap_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/systemtest/sourcemap_test.go b/systemtest/sourcemap_test.go index 1378325875d..864bbf0a11f 100644 --- a/systemtest/sourcemap_test.go +++ b/systemtest/sourcemap_test.go @@ -141,6 +141,7 @@ func TestSourcemapCaching(t *testing.T) { srv := apmservertest.NewUnstartedServerTB(t) srv.Config.RUM = &apmservertest.RUMConfig{Enabled: true} + srv.Config.Kibana.Enabled = false err = srv.Start() require.NoError(t, err) From 82f28b425c7888812598aae467a740928052df95 Mon Sep 17 00:00:00 2001 From: kruskal <99559985+kruskall@users.noreply.github.com> Date: Mon, 27 Feb 2023 02:59:09 +0100 Subject: [PATCH 02/13] fix: fall back immediately if the metadata fetcher is not ready --- internal/sourcemap/metadata_fetcher.go | 8 ++++---- internal/sourcemap/sourcemap_fetcher.go | 2 ++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/internal/sourcemap/metadata_fetcher.go b/internal/sourcemap/metadata_fetcher.go index 1ae6189adef..fc4bad3d63f 100644 --- a/internal/sourcemap/metadata_fetcher.go +++ b/internal/sourcemap/metadata_fetcher.go @@ -101,9 +101,9 @@ func (s *MetadataESFetcher) startBackgroundSync(parent context.Context) { go func() { s.logger.Debug("populating metadata cache") - ctx, cancel := context.WithTimeout(parent, 1*time.Second) - err := s.ping(ctx) - cancel() + // the parent ctx does not have a deadline so we + // rely on the es client timeout (default 5s). + err := s.ping(parent) if err != nil { // it is fine to not lock here since err will not access @@ -112,7 +112,7 @@ func (s *MetadataESFetcher) startBackgroundSync(parent context.Context) { s.logger.Error(s.initErr) } else { // First run, populate cache - ctx, cancel = context.WithTimeout(parent, syncTimeout) + ctx, cancel := context.WithTimeout(parent, syncTimeout) err := s.sync(ctx) cancel() diff --git a/internal/sourcemap/sourcemap_fetcher.go b/internal/sourcemap/sourcemap_fetcher.go index 6f5e7f350db..9b5b8700e61 100644 --- a/internal/sourcemap/sourcemap_fetcher.go +++ b/internal/sourcemap/sourcemap_fetcher.go @@ -54,6 +54,8 @@ func (s *SourcemapFetcher) Fetch(ctx context.Context, name, version, path string } case <-ctx.Done(): return nil, fmt.Errorf("error waiting for metadata fetcher to be ready: %w", ctx.Err()) + default: + return nil, fmt.Errorf("metadata fetcher is not ready: %w", errFetcherUnvailable) } if i, ok := s.metadata.getID(original); ok { From 3ea51eed20b2648b3a88394790ee37d300e9bd55 Mon Sep 17 00:00:00 2001 From: kruskal <99559985+kruskall@users.noreply.github.com> Date: Tue, 28 Feb 2023 01:20:08 +0100 Subject: [PATCH 03/13] test: add back delete hook for sourcemaps using the kibana api --- systemtest/elasticsearch.go | 10 ---------- systemtest/kibana.go | 22 ++++++++++++++++++++++ 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/systemtest/elasticsearch.go b/systemtest/elasticsearch.go index a2c94e4d775..5b79713b34c 100644 --- a/systemtest/elasticsearch.go +++ b/systemtest/elasticsearch.go @@ -20,7 +20,6 @@ package systemtest import ( "context" "net/url" - "strings" "testing" "time" @@ -102,15 +101,6 @@ func cleanupElasticsearch() error { }, ExpandWildcards: "all", }, nil) - if err != nil { - return err - } - - _, err = Elasticsearch.Do(context.Background(), &esapi.DeleteByQueryRequest{ - Index: []string{".apm-source-map"}, - Body: strings.NewReader(`{"query": { "match_all": {}}}`), - Conflicts: "proceed", - }, nil) return err } diff --git a/systemtest/kibana.go b/systemtest/kibana.go index ec84ceb795b..5e623ffea91 100644 --- a/systemtest/kibana.go +++ b/systemtest/kibana.go @@ -314,5 +314,27 @@ func CreateSourceMap(t testing.TB, sourcemap []byte, serviceName, serviceVersion Value: id, }) + t.Cleanup(func() { + DeleteSourceMap(t, result.ID) + }) + return result.ID } + +// DeleteSourceMap deletes a source map with the given ID. +func DeleteSourceMap(t testing.TB, id string) { + t.Helper() + + url := *KibanaURL + url.Path += "/api/apm/sourcemaps/" + id + req, _ := http.NewRequest("DELETE", url.String(), nil) + req.Header.Set("kbn-xsrf", "1") + + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + respBody, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode, string(respBody)) +} From 61091fba47ca39d147b5a0bd48afda55045d151e Mon Sep 17 00:00:00 2001 From: kruskal <99559985+kruskall@users.noreply.github.com> Date: Tue, 28 Feb 2023 01:21:20 +0100 Subject: [PATCH 04/13] refactor: remove arbitrary 500ms timeout in chained fetcher --- internal/sourcemap/chained.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/internal/sourcemap/chained.go b/internal/sourcemap/chained.go index af055c2b04a..61b5d761aa2 100644 --- a/internal/sourcemap/chained.go +++ b/internal/sourcemap/chained.go @@ -20,7 +20,6 @@ package sourcemap import ( "context" "errors" - "time" "github.com/go-sourcemap/sourcemap" ) @@ -44,13 +43,6 @@ func (c ChainedFetcher) Fetch(ctx context.Context, name, version, path string) ( return consumer, err } - // previous fetcher is unavailable but the deadline expired so we cannot reuse that - if t, _ := ctx.Deadline(); t.Before(time.Now()) { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(context.Background(), 500*time.Millisecond) - defer cancel() - } - // err is errFetcherUnvailable // store it in a tmp variable and try the next fetcher lastErr = err From 4798579f857cd66abbbf5f3f7efe6d4e9e725dee Mon Sep 17 00:00:00 2001 From: kruskal <99559985+kruskall@users.noreply.github.com> Date: Tue, 28 Feb 2023 01:21:53 +0100 Subject: [PATCH 05/13] test: disable kibana in nomatching sourcemap test --- systemtest/sourcemap_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/systemtest/sourcemap_test.go b/systemtest/sourcemap_test.go index 864bbf0a11f..bb0abe6b4dd 100644 --- a/systemtest/sourcemap_test.go +++ b/systemtest/sourcemap_test.go @@ -112,6 +112,7 @@ func TestNoMatchingSourcemap(t *testing.T) { srv := apmservertest.NewUnstartedServerTB(t) srv.Config.RUM = &apmservertest.RUMConfig{Enabled: true} + srv.Config.Kibana.Enabled = false err = srv.Start() require.NoError(t, err) From 4376e012b5329552fc0768442a9770839e018fff Mon Sep 17 00:00:00 2001 From: kruskal <99559985+kruskall@users.noreply.github.com> Date: Tue, 28 Feb 2023 01:30:42 +0100 Subject: [PATCH 06/13] test: update approvals results --- systemtest/approvals/TestNoMatchingSourcemap.approved.json | 6 ------ .../approvals/TestRUMRoutingIntegration.approved.json | 6 ------ 2 files changed, 12 deletions(-) diff --git a/systemtest/approvals/TestNoMatchingSourcemap.approved.json b/systemtest/approvals/TestNoMatchingSourcemap.approved.json index 9e0217a1de0..b975d6c1b5f 100644 --- a/systemtest/approvals/TestNoMatchingSourcemap.approved.json +++ b/systemtest/approvals/TestNoMatchingSourcemap.approved.json @@ -51,9 +51,6 @@ "line": { "column": 18, "number": 1 - }, - "sourcemap": { - "error": "unable to find sourcemap.url for service.name=apm-agent-js service.version=1.0.0 bundle.path=http://subdomain1.localhost:8000/test/e2e/general-usecase/bundle.js.map" } }, { @@ -65,9 +62,6 @@ "line": { "column": 18, "number": 1 - }, - "sourcemap": { - "error": "unable to find sourcemap.url for service.name=apm-agent-js service.version=1.0.0 bundle.path=http://subdomain2.localhost:8000/test/e2e/general-usecase/bundle.js.map" } } ], diff --git a/systemtest/approvals/TestRUMRoutingIntegration.approved.json b/systemtest/approvals/TestRUMRoutingIntegration.approved.json index 791dc75026e..1ec96e24e9d 100644 --- a/systemtest/approvals/TestRUMRoutingIntegration.approved.json +++ b/systemtest/approvals/TestRUMRoutingIntegration.approved.json @@ -437,9 +437,6 @@ "line": { "column": 9, "number": 7662 - }, - "sourcemap": { - "error": "unable to find sourcemap.url for service.name=apm-a-rum-test-e2e-general-usecase service.version=0.0.1 bundle.path=http://localhost:8000/test/e2e/general-usecase/app.e2e-bundle.min.js" } }, { @@ -450,9 +447,6 @@ "line": { "column": 3, "number": 7666 - }, - "sourcemap": { - "error": "unable to find sourcemap.url for service.name=apm-a-rum-test-e2e-general-usecase service.version=0.0.1 bundle.path=http://localhost:8000/test/e2e/general-usecase/app.e2e-bundle.min.js" } } ], From 50bcd879ab108d5ca04eafd267bafd5286ab84cd Mon Sep 17 00:00:00 2001 From: kruskal <99559985+kruskall@users.noreply.github.com> Date: Tue, 28 Feb 2023 01:46:40 +0100 Subject: [PATCH 07/13] test: add time sleep to wait for init --- internal/beater/beater_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/beater/beater_test.go b/internal/beater/beater_test.go index 8ded9e68068..67d9b0f3e06 100644 --- a/internal/beater/beater_test.go +++ b/internal/beater/beater_test.go @@ -28,6 +28,7 @@ import ( "net/http/httptest" "os" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -76,6 +77,9 @@ func TestStoreUsesRUMElasticsearchConfig(t *testing.T) { ) require.NoError(t, err) defer cancel() + + time.Sleep(1 * time.Second) + // Check that the provided rum elasticsearch config was used and // Fetch() goes to the test server. c, err := fetcher.Fetch(context.Background(), "app", "1.0", "/bundle/path") From 738898bf46ab705b58cfa12c47fd326cde1ea5a9 Mon Sep 17 00:00:00 2001 From: kruskal <99559985+kruskall@users.noreply.github.com> Date: Wed, 1 Mar 2023 00:32:34 +0100 Subject: [PATCH 08/13] refactor: avoid time sleep in tests --- internal/beater/beater_test.go | 3 +-- internal/sourcemap/body_caching.go | 4 ++++ internal/sourcemap/chained.go | 7 +++++++ internal/sourcemap/elasticsearch.go | 4 ++++ internal/sourcemap/fetcher.go | 2 ++ internal/sourcemap/kibana.go | 3 +++ internal/sourcemap/sourcemap_fetcher.go | 5 +++++ internal/sourcemap/sourcemap_fetcher_test.go | 3 +++ 8 files changed, 29 insertions(+), 2 deletions(-) diff --git a/internal/beater/beater_test.go b/internal/beater/beater_test.go index 67d9b0f3e06..c27af99ed4c 100644 --- a/internal/beater/beater_test.go +++ b/internal/beater/beater_test.go @@ -28,7 +28,6 @@ import ( "net/http/httptest" "os" "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -78,7 +77,7 @@ func TestStoreUsesRUMElasticsearchConfig(t *testing.T) { require.NoError(t, err) defer cancel() - time.Sleep(1 * time.Second) + fetcher.Wait() // Check that the provided rum elasticsearch config was used and // Fetch() goes to the test server. diff --git a/internal/sourcemap/body_caching.go b/internal/sourcemap/body_caching.go index 75eacee1f60..d4522a6bc77 100644 --- a/internal/sourcemap/body_caching.go +++ b/internal/sourcemap/body_caching.go @@ -100,3 +100,7 @@ func (s *BodyCachingFetcher) add(key identifier, consumer *sourcemap.Consumer) { s.cache.Add(key, consumer) s.logger.Debugf("Added id %v. Cache now has %v entries.", key, s.cache.Len()) } + + +func (s *BodyCachingFetcher) Wait() { +} diff --git a/internal/sourcemap/chained.go b/internal/sourcemap/chained.go index 61b5d761aa2..35639fbf014 100644 --- a/internal/sourcemap/chained.go +++ b/internal/sourcemap/chained.go @@ -49,3 +49,10 @@ func (c ChainedFetcher) Fetch(ctx context.Context, name, version, path string) ( } return nil, lastErr } + + +func (c ChainedFetcher) Wait() { + for _, v := range c { + v.Wait() + } +} diff --git a/internal/sourcemap/elasticsearch.go b/internal/sourcemap/elasticsearch.go index d0a1139b285..78ff5d6ff4f 100644 --- a/internal/sourcemap/elasticsearch.go +++ b/internal/sourcemap/elasticsearch.go @@ -144,3 +144,7 @@ func parse(body io.ReadCloser, name, version, path string, logger *logp.Logger) return esSourcemapResponse.Source.Sourcemap, nil } + + +func (s *esFetcher) Wait() { +} diff --git a/internal/sourcemap/fetcher.go b/internal/sourcemap/fetcher.go index ea9c6495d94..cc911bfa55e 100644 --- a/internal/sourcemap/fetcher.go +++ b/internal/sourcemap/fetcher.go @@ -38,6 +38,8 @@ type Fetcher interface { // // If there is no such source map available, Fetch returns a nil Consumer. Fetch(ctx context.Context, name string, version string, bundleFilepath string) (*sourcemap.Consumer, error) + + Wait() } // MetadataFetcher is an interface for fetching metadata diff --git a/internal/sourcemap/kibana.go b/internal/sourcemap/kibana.go index 1e7e1660cea..3b78cac571a 100644 --- a/internal/sourcemap/kibana.go +++ b/internal/sourcemap/kibana.go @@ -86,6 +86,9 @@ func (s *kibanaFetcher) Fetch(ctx context.Context, name, version, path string) ( return nil, nil } +func (s *kibanaFetcher) Wait() { +} + // maybeParseURLPath attempts to parse s as a URL, returning its path component // if successful. If s cannot be parsed as a URL, s is returned. func maybeParseURLPath(s string) string { diff --git a/internal/sourcemap/sourcemap_fetcher.go b/internal/sourcemap/sourcemap_fetcher.go index 9b5b8700e61..4f64769ba0d 100644 --- a/internal/sourcemap/sourcemap_fetcher.go +++ b/internal/sourcemap/sourcemap_fetcher.go @@ -105,3 +105,8 @@ func (s *SourcemapFetcher) fetch(ctx context.Context, key *identifier) (*sourcem return c, err } + + +func (s *SourcemapFetcher) Wait() { + <-s.metadata.ready() +} diff --git a/internal/sourcemap/sourcemap_fetcher_test.go b/internal/sourcemap/sourcemap_fetcher_test.go index 1d69e8f6a55..987f7915679 100644 --- a/internal/sourcemap/sourcemap_fetcher_test.go +++ b/internal/sourcemap/sourcemap_fetcher_test.go @@ -133,3 +133,6 @@ func (s *monitoredFetcher) Fetch(ctx context.Context, name string, version strin return &sourcemap.Consumer{}, nil } + +func (s *monitoredFetcher) Wait() { +} From c190a08e4988f67641508a301f5e1a791f35a472 Mon Sep 17 00:00:00 2001 From: kruskal <99559985+kruskall@users.noreply.github.com> Date: Wed, 1 Mar 2023 00:49:52 +0100 Subject: [PATCH 09/13] refactor: remove es ping and rely on initial sync request --- internal/sourcemap/metadata_fetcher.go | 43 ++++---------------------- 1 file changed, 6 insertions(+), 37 deletions(-) diff --git a/internal/sourcemap/metadata_fetcher.go b/internal/sourcemap/metadata_fetcher.go index fc4bad3d63f..ded894f6897 100644 --- a/internal/sourcemap/metadata_fetcher.go +++ b/internal/sourcemap/metadata_fetcher.go @@ -101,31 +101,13 @@ func (s *MetadataESFetcher) startBackgroundSync(parent context.Context) { go func() { s.logger.Debug("populating metadata cache") - // the parent ctx does not have a deadline so we - // rely on the es client timeout (default 5s). - err := s.ping(parent) - - if err != nil { - // it is fine to not lock here since err will not access - // initErr until the init channel is closed. - s.initErr = fmt.Errorf("failed to ping es cluster: %w: %v", errFetcherUnvailable, err) + // First run, populate cache + ctx, cancel := context.WithTimeout(parent, syncTimeout) + if err := s.sync(ctx); err != nil { + s.initErr = fmt.Errorf("failed to populate sourcemap metadata: %w", err) s.logger.Error(s.initErr) - } else { - // First run, populate cache - ctx, cancel := context.WithTimeout(parent, syncTimeout) - err := s.sync(ctx) - cancel() - - s.initErr = err - - if err != nil { - s.logger.Errorf("failed to fetch sourcemaps metadata: %v", err) - } else { - // only close the init chan and mark the fetcher as ready if - // sync succeeded - s.logger.Info("init routine completed") - } } + cancel() close(s.init) @@ -152,19 +134,6 @@ func (s *MetadataESFetcher) startBackgroundSync(parent context.Context) { }() } -func (s *MetadataESFetcher) ping(ctx context.Context) error { - // we cannot use PingRequest because the library is - // building a broken url and the request is timing out. - req := esapi.IndicesGetRequest{ - Index: []string{s.index}, - } - resp, err := req.Do(ctx, s.esClient) - if err == nil { - resp.Body.Close() - } - return err -} - func (s *MetadataESFetcher) sync(ctx context.Context) error { sourcemaps := make(map[identifier]string) @@ -264,7 +233,7 @@ func (s *MetadataESFetcher) update(ctx context.Context, sourcemaps map[identifie func (s *MetadataESFetcher) initialSearch(ctx context.Context, updates map[identifier]string) (*esSearchSourcemapResponse, error) { resp, err := s.runSearchQuery(ctx) if err != nil { - return nil, fmt.Errorf("failed to run initial search query: %w", err) + return nil, fmt.Errorf("failed to run initial search query: %w: %v", errFetcherUnvailable, err) } defer resp.Body.Close() From 5ee25bbc458e9092a1b75af895180fcb6596a8a2 Mon Sep 17 00:00:00 2001 From: kruskal <99559985+kruskall@users.noreply.github.com> Date: Wed, 1 Mar 2023 00:51:01 +0100 Subject: [PATCH 10/13] refactor: remove arbitrary 10s timeout and rely on es client timeout --- internal/sourcemap/metadata_fetcher.go | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/internal/sourcemap/metadata_fetcher.go b/internal/sourcemap/metadata_fetcher.go index ded894f6897..285a317bc5d 100644 --- a/internal/sourcemap/metadata_fetcher.go +++ b/internal/sourcemap/metadata_fetcher.go @@ -33,10 +33,6 @@ import ( "github.com/elastic/go-elasticsearch/v8/esapi" ) -const ( - syncTimeout = 10 * time.Second -) - type MetadataESFetcher struct { esClient *elasticsearch.Client index string @@ -97,17 +93,15 @@ func (s *MetadataESFetcher) err() error { } } -func (s *MetadataESFetcher) startBackgroundSync(parent context.Context) { +func (s *MetadataESFetcher) startBackgroundSync(ctx context.Context) { go func() { s.logger.Debug("populating metadata cache") // First run, populate cache - ctx, cancel := context.WithTimeout(parent, syncTimeout) if err := s.sync(ctx); err != nil { s.initErr = fmt.Errorf("failed to populate sourcemap metadata: %w", err) s.logger.Error(s.initErr) } - cancel() close(s.init) @@ -117,14 +111,10 @@ func (s *MetadataESFetcher) startBackgroundSync(parent context.Context) { for { select { case <-t.C: - ctx, cancel := context.WithTimeout(parent, syncTimeout) - if err := s.sync(ctx); err != nil { s.logger.Errorf("failed to sync sourcemaps metadata: %v", err) } - - cancel() - case <-parent.Done(): + case <-ctx.Done(): s.logger.Info("update routine done") // close invalidation channel close(s.invalidationChan) From 082ba548d80484589fc658be7a6ae0aaab2a1f1d Mon Sep 17 00:00:00 2001 From: kruskal <99559985+kruskall@users.noreply.github.com> Date: Wed, 1 Mar 2023 01:08:33 +0100 Subject: [PATCH 11/13] lint: fix linter issues --- internal/sourcemap/body_caching.go | 1 - internal/sourcemap/chained.go | 1 - internal/sourcemap/elasticsearch.go | 1 - internal/sourcemap/sourcemap_fetcher.go | 1 - 4 files changed, 4 deletions(-) diff --git a/internal/sourcemap/body_caching.go b/internal/sourcemap/body_caching.go index d4522a6bc77..b0cc9eae81d 100644 --- a/internal/sourcemap/body_caching.go +++ b/internal/sourcemap/body_caching.go @@ -101,6 +101,5 @@ func (s *BodyCachingFetcher) add(key identifier, consumer *sourcemap.Consumer) { s.logger.Debugf("Added id %v. Cache now has %v entries.", key, s.cache.Len()) } - func (s *BodyCachingFetcher) Wait() { } diff --git a/internal/sourcemap/chained.go b/internal/sourcemap/chained.go index 35639fbf014..e9baf0f8823 100644 --- a/internal/sourcemap/chained.go +++ b/internal/sourcemap/chained.go @@ -50,7 +50,6 @@ func (c ChainedFetcher) Fetch(ctx context.Context, name, version, path string) ( return nil, lastErr } - func (c ChainedFetcher) Wait() { for _, v := range c { v.Wait() diff --git a/internal/sourcemap/elasticsearch.go b/internal/sourcemap/elasticsearch.go index 78ff5d6ff4f..56e2c884e3f 100644 --- a/internal/sourcemap/elasticsearch.go +++ b/internal/sourcemap/elasticsearch.go @@ -145,6 +145,5 @@ func parse(body io.ReadCloser, name, version, path string, logger *logp.Logger) return esSourcemapResponse.Source.Sourcemap, nil } - func (s *esFetcher) Wait() { } diff --git a/internal/sourcemap/sourcemap_fetcher.go b/internal/sourcemap/sourcemap_fetcher.go index 4f64769ba0d..3448f8fe2cd 100644 --- a/internal/sourcemap/sourcemap_fetcher.go +++ b/internal/sourcemap/sourcemap_fetcher.go @@ -106,7 +106,6 @@ func (s *SourcemapFetcher) fetch(ctx context.Context, key *identifier) (*sourcem return c, err } - func (s *SourcemapFetcher) Wait() { <-s.metadata.ready() } From 6870b2d054449d3eb3e903524093e5461f4150c6 Mon Sep 17 00:00:00 2001 From: kruskal <99559985+kruskall@users.noreply.github.com> Date: Wed, 1 Mar 2023 03:52:13 +0100 Subject: [PATCH 12/13] test: refactor es config test and remove wait method from interface --- internal/beater/beater_test.go | 28 +++++++------------- internal/sourcemap/body_caching.go | 3 --- internal/sourcemap/chained.go | 6 ----- internal/sourcemap/elasticsearch.go | 3 --- internal/sourcemap/fetcher.go | 2 -- internal/sourcemap/kibana.go | 3 --- internal/sourcemap/sourcemap_fetcher.go | 4 --- internal/sourcemap/sourcemap_fetcher_test.go | 3 --- 8 files changed, 9 insertions(+), 43 deletions(-) diff --git a/internal/beater/beater_test.go b/internal/beater/beater_test.go index c27af99ed4c..f12ced372b7 100644 --- a/internal/beater/beater_test.go +++ b/internal/beater/beater_test.go @@ -28,6 +28,7 @@ import ( "net/http/httptest" "os" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -40,22 +41,15 @@ import ( var validSourcemap, _ = os.ReadFile("../../testdata/sourcemap/bundle.js.map") func TestStoreUsesRUMElasticsearchConfig(t *testing.T) { - var called bool + initCh := make(chan struct{}) ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("X-Elastic-Product", "Elasticsearch") switch r.URL.Path { - case "/.apm-source-map": - // ping request from the metadata fetcher. - // Send a status ok - w.WriteHeader(http.StatusOK) case "/.apm-source-map/_search": // search request from the metadata fetcher m := sourcemapSearchResponseBody("app", "1.0", "/bundle/path") w.Write(m) - case "/.apm-source-map/_doc/app-1.0-/bundle/path": - m := sourcemapGetResponseBody(true, validSourcemap) - w.Write(m) - called = true + close(initCh) default: w.WriteHeader(http.StatusTeapot) t.Fatalf("unhandled request path: %s", r.URL.Path) @@ -70,22 +64,18 @@ func TestStoreUsesRUMElasticsearchConfig(t *testing.T) { cfg.RumConfig.SourceMapping.ESConfig = elasticsearch.DefaultConfig() cfg.RumConfig.SourceMapping.ESConfig.Hosts = []string{ts.URL} - fetcher, cancel, err := newSourcemapFetcher( + _, cancel, err := newSourcemapFetcher( cfg.RumConfig.SourceMapping, nil, elasticsearch.NewClient, ) require.NoError(t, err) defer cancel() - fetcher.Wait() - - // Check that the provided rum elasticsearch config was used and - // Fetch() goes to the test server. - c, err := fetcher.Fetch(context.Background(), "app", "1.0", "/bundle/path") - require.NoError(t, err) - require.NotNil(t, c) - - assert.True(t, called) + select { + case <-initCh: + case <-time.After(10 * time.Second): + t.Fatal("timed out waiting for metadata fetcher init to complete") + } } func sourcemapSearchResponseBody(name string, version string, bundlePath string) []byte { diff --git a/internal/sourcemap/body_caching.go b/internal/sourcemap/body_caching.go index b0cc9eae81d..75eacee1f60 100644 --- a/internal/sourcemap/body_caching.go +++ b/internal/sourcemap/body_caching.go @@ -100,6 +100,3 @@ func (s *BodyCachingFetcher) add(key identifier, consumer *sourcemap.Consumer) { s.cache.Add(key, consumer) s.logger.Debugf("Added id %v. Cache now has %v entries.", key, s.cache.Len()) } - -func (s *BodyCachingFetcher) Wait() { -} diff --git a/internal/sourcemap/chained.go b/internal/sourcemap/chained.go index e9baf0f8823..61b5d761aa2 100644 --- a/internal/sourcemap/chained.go +++ b/internal/sourcemap/chained.go @@ -49,9 +49,3 @@ func (c ChainedFetcher) Fetch(ctx context.Context, name, version, path string) ( } return nil, lastErr } - -func (c ChainedFetcher) Wait() { - for _, v := range c { - v.Wait() - } -} diff --git a/internal/sourcemap/elasticsearch.go b/internal/sourcemap/elasticsearch.go index 56e2c884e3f..d0a1139b285 100644 --- a/internal/sourcemap/elasticsearch.go +++ b/internal/sourcemap/elasticsearch.go @@ -144,6 +144,3 @@ func parse(body io.ReadCloser, name, version, path string, logger *logp.Logger) return esSourcemapResponse.Source.Sourcemap, nil } - -func (s *esFetcher) Wait() { -} diff --git a/internal/sourcemap/fetcher.go b/internal/sourcemap/fetcher.go index cc911bfa55e..ea9c6495d94 100644 --- a/internal/sourcemap/fetcher.go +++ b/internal/sourcemap/fetcher.go @@ -38,8 +38,6 @@ type Fetcher interface { // // If there is no such source map available, Fetch returns a nil Consumer. Fetch(ctx context.Context, name string, version string, bundleFilepath string) (*sourcemap.Consumer, error) - - Wait() } // MetadataFetcher is an interface for fetching metadata diff --git a/internal/sourcemap/kibana.go b/internal/sourcemap/kibana.go index 3b78cac571a..1e7e1660cea 100644 --- a/internal/sourcemap/kibana.go +++ b/internal/sourcemap/kibana.go @@ -86,9 +86,6 @@ func (s *kibanaFetcher) Fetch(ctx context.Context, name, version, path string) ( return nil, nil } -func (s *kibanaFetcher) Wait() { -} - // maybeParseURLPath attempts to parse s as a URL, returning its path component // if successful. If s cannot be parsed as a URL, s is returned. func maybeParseURLPath(s string) string { diff --git a/internal/sourcemap/sourcemap_fetcher.go b/internal/sourcemap/sourcemap_fetcher.go index 3448f8fe2cd..9b5b8700e61 100644 --- a/internal/sourcemap/sourcemap_fetcher.go +++ b/internal/sourcemap/sourcemap_fetcher.go @@ -105,7 +105,3 @@ func (s *SourcemapFetcher) fetch(ctx context.Context, key *identifier) (*sourcem return c, err } - -func (s *SourcemapFetcher) Wait() { - <-s.metadata.ready() -} diff --git a/internal/sourcemap/sourcemap_fetcher_test.go b/internal/sourcemap/sourcemap_fetcher_test.go index 987f7915679..1d69e8f6a55 100644 --- a/internal/sourcemap/sourcemap_fetcher_test.go +++ b/internal/sourcemap/sourcemap_fetcher_test.go @@ -133,6 +133,3 @@ func (s *monitoredFetcher) Fetch(ctx context.Context, name string, version strin return &sourcemap.Consumer{}, nil } - -func (s *monitoredFetcher) Wait() { -} From 6406c88116e4ef9714d16edcb935761e6509eef1 Mon Sep 17 00:00:00 2001 From: kruskal <99559985+kruskall@users.noreply.github.com> Date: Wed, 1 Mar 2023 04:17:36 +0100 Subject: [PATCH 13/13] lint: remove unused methods --- internal/beater/beater_test.go | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/internal/beater/beater_test.go b/internal/beater/beater_test.go index f12ced372b7..ace62d467a8 100644 --- a/internal/beater/beater_test.go +++ b/internal/beater/beater_test.go @@ -18,15 +18,11 @@ package beater import ( - "bytes" - "compress/zlib" "context" - "encoding/base64" "encoding/json" "fmt" "net/http" "net/http/httptest" - "os" "testing" "time" @@ -38,8 +34,6 @@ import ( "github.com/elastic/elastic-agent-libs/monitoring" ) -var validSourcemap, _ = os.ReadFile("../../testdata/sourcemap/bundle.js.map") - func TestStoreUsesRUMElasticsearchConfig(t *testing.T) { initCh := make(chan struct{}) ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -107,31 +101,6 @@ func sourcemapSearchResponseBody(name string, version string, bundlePath string) return data } -func sourcemapGetResponseBody(found bool, b []byte) []byte { - result := map[string]interface{}{ - "found": found, - "_source": map[string]interface{}{ - "content": encodeSourcemap(b), - }, - } - - data, err := json.Marshal(result) - if err != nil { - panic(err) - } - return data -} - -func encodeSourcemap(sourcemap []byte) string { - b := &bytes.Buffer{} - - z := zlib.NewWriter(b) - z.Write(sourcemap) - z.Close() - - return base64.StdEncoding.EncodeToString(b.Bytes()) -} - func TestQueryClusterUUIDRegistriesExist(t *testing.T) { stateRegistry := monitoring.GetNamespace("state").GetRegistry() stateRegistry.Clear()