From ee24fea7759448f4a44bd1b4166952a2319a57d5 Mon Sep 17 00:00:00 2001 From: Gil Raphaelli Date: Tue, 6 Aug 2019 18:37:15 -0400 Subject: [PATCH] Disable sourcemapping config (#2488) (#2491) --- _meta/beat.yml | 3 +++ apm-server.docker.yml | 3 +++ apm-server.yml | 3 +++ beater/config.go | 7 ++++- beater/route_config.go | 2 +- changelogs/7.4.asciidoc | 1 + tests/system/apmserver.py | 10 ++------ tests/system/config/apm-server.yml.j2 | 8 ++++-- tests/system/test_integration.py | 37 ++++++++++++++++++++++++--- 9 files changed, 58 insertions(+), 16 deletions(-) diff --git a/_meta/beat.yml b/_meta/beat.yml index d2e9d521a61..a64f7eb3a03 100644 --- a/_meta/beat.yml +++ b/_meta/beat.yml @@ -112,6 +112,9 @@ apm-server: # to all error and transaction documents sent to the RUM endpoint. #source_mapping: + # Sourcemapping is enabled by default for RUM events + #enabled: true + # Source maps are always fetched from Elasticsearch, by default using the output.elasticsearch configuration. # A different instance must be configured when using any other output. # This setting only affects sourcemap reads - the output determines where sourcemaps are written. diff --git a/apm-server.docker.yml b/apm-server.docker.yml index 90f44fd83d6..55cfd21a941 100644 --- a/apm-server.docker.yml +++ b/apm-server.docker.yml @@ -112,6 +112,9 @@ apm-server: # to all error and transaction documents sent to the RUM endpoint. #source_mapping: + # Sourcemapping is enabled by default for RUM events + #enabled: true + # Source maps are always fetched from Elasticsearch, by default using the output.elasticsearch configuration. # A different instance must be configured when using any other output. # This setting only affects sourcemap reads - the output determines where sourcemaps are written. diff --git a/apm-server.yml b/apm-server.yml index 8acb9fbaaa9..d6dfc943d14 100644 --- a/apm-server.yml +++ b/apm-server.yml @@ -112,6 +112,9 @@ apm-server: # to all error and transaction documents sent to the RUM endpoint. #source_mapping: + # Sourcemapping is enabled by default for RUM events + #enabled: true + # Source maps are always fetched from Elasticsearch, by default using the output.elasticsearch configuration. # A different instance must be configured when using any other output. # This setting only affects sourcemap reads - the output determines where sourcemaps are written. diff --git a/beater/config.go b/beater/config.go index 215cdd7f94d..2d4bb029df3 100644 --- a/beater/config.go +++ b/beater/config.go @@ -106,6 +106,7 @@ type agentConfig struct { type SourceMapping struct { Cache *Cache `config:"cache"` + Enabled *bool `config:"enabled"` IndexPattern string `config:"index_pattern"` EsConfig *common.Config `config:"elasticsearch"` @@ -188,6 +189,10 @@ func (c *rumConfig) isEnabled() bool { return c != nil && (c.Enabled != nil && *c.Enabled) } +func (s *SourceMapping) isEnabled() bool { + return s == nil || s.Enabled == nil || *s.Enabled +} + func (s *SourceMapping) isSetup() bool { return s != nil && (s.EsConfig != nil) } @@ -201,7 +206,7 @@ func (c *pipelineConfig) shouldOverwrite() bool { } func (c *rumConfig) memoizedSmapMapper() (sourcemap.Mapper, error) { - if !c.isEnabled() || !c.SourceMapping.isSetup() { + if !c.isEnabled() || !c.SourceMapping.isEnabled() || !c.SourceMapping.isSetup() { return nil, nil } if c.SourceMapping.mapper != nil { diff --git a/beater/route_config.go b/beater/route_config.go index 065b7cb9b70..f080a937919 100644 --- a/beater/route_config.go +++ b/beater/route_config.go @@ -146,7 +146,7 @@ func rumHandler(beaterConfig *Config, h http.Handler) http.Handler { func sourcemapHandler(beaterConfig *Config, h http.Handler) http.Handler { return logHandler( - killSwitchHandler(beaterConfig.RumConfig.isEnabled(), + killSwitchHandler(beaterConfig.RumConfig.isEnabled() && beaterConfig.RumConfig.SourceMapping.isEnabled(), authHandler(beaterConfig.SecretToken, h))) } diff --git a/changelogs/7.4.asciidoc b/changelogs/7.4.asciidoc index af111c762a3..6d5556578a3 100644 --- a/changelogs/7.4.asciidoc +++ b/changelogs/7.4.asciidoc @@ -13,3 +13,4 @@ https://github.com/elastic/apm-server/compare/v7.3.0\...v7.4.0[View commits] [float] ==== Added - Upgrade Go to 1.12.7 {pull}2483[2483]. +- Add config option to disable sourcemapping {pull}2488[2488]. diff --git a/tests/system/apmserver.py b/tests/system/apmserver.py index 619e508e83b..07a28ceaec7 100644 --- a/tests/system/apmserver.py +++ b/tests/system/apmserver.py @@ -260,6 +260,7 @@ class ClientSideBaseTest(ServerBaseTest): sourcemap_url = 'http://localhost:8200/assets/v1/sourcemaps' intake_url = 'http://localhost:8200/intake/v2/rum/events' backend_intake_url = 'http://localhost:8200/intake/v2/events' + config_overrides = {} @classmethod def setUpClass(cls): @@ -269,6 +270,7 @@ def config(self): cfg = super(ClientSideBaseTest, self).config() cfg.update({"enable_rum": "true", "smap_cache_expiration": "200"}) + cfg.update(self.config_overrides) return cfg def get_backend_error_payload_path(self, name="errors_2.ndjson"): @@ -367,20 +369,12 @@ def tearDown(self): class CorsBaseTest(ClientSideBaseTest): - def config(self): cfg = super(CorsBaseTest, self).config() cfg.update({"allow_origins": ["http://www.elastic.co"]}) return cfg -class SmapCacheBaseTest(ClientSideElasticTest): - def config(self): - cfg = super(SmapCacheBaseTest, self).config() - cfg.update({"smap_cache_expiration": "1"}) - return cfg - - class ExpvarBaseTest(ServerBaseTest): config_overrides = {} diff --git a/tests/system/config/apm-server.yml.j2 b/tests/system/config/apm-server.yml.j2 index 3d81d7fc7d2..a61c5e70966 100644 --- a/tests/system/config/apm-server.yml.j2 +++ b/tests/system/config/apm-server.yml.j2 @@ -48,10 +48,14 @@ apm-server: rum.exclude_from_grouping: "~/test" rum.event_rate.limit: 16 - {% if smap_cache_expiration%} + {% if rum_sourcemapping_disabled %} + rum.source_mapping.enabled: false + {% endif %} + + {% if smap_cache_expiration %} rum.source_mapping.cache.expiration: {{ smap_cache_expiration}} {% endif %} - {% if smap_index_pattern%} + {% if smap_index_pattern %} rum.source_mapping.index_pattern: {{ smap_index_pattern}} {% endif %} diff --git a/tests/system/test_integration.py b/tests/system/test_integration.py index 4b76e24773d..87206f61592 100644 --- a/tests/system/test_integration.py +++ b/tests/system/test_integration.py @@ -4,7 +4,7 @@ import unittest from apmserver import ElasticTest, ExpvarBaseTest -from apmserver import ClientSideElasticTest, SmapCacheBaseTest +from apmserver import ClientSideElasticTest from apmserver import OverrideIndicesTest, OverrideIndicesFailureTest from beat.beat import INTEGRATION_TESTS from sets import Set @@ -505,8 +505,6 @@ def test_sourcemap_mapping_cache_usage(self): self.assert_no_logged_warnings() self.check_rum_error_sourcemap(True) - -class SourcemappingIntegrationChangedConfigTest(ClientSideElasticTest): @unittest.skipUnless(INTEGRATION_TESTS, "integration test") def test_rum_error_changed_index(self): # use an uncleaned path to test that path is cleaned in upload @@ -524,7 +522,9 @@ def test_rum_error_changed_index(self): self.check_rum_error_sourcemap(True) -class SourcemappingCacheIntegrationTest(SmapCacheBaseTest): +class SourcemappingCacheIntegrationTest(ClientSideElasticTest): + config_overrides = {"smap_cache_expiration": "1"} + @unittest.skipUnless(INTEGRATION_TESTS, "integration test") def test_sourcemap_cache_expiration(self): path = 'http://localhost:8000/test/e2e/general-usecase/bundle.js.map' @@ -556,6 +556,35 @@ def test_sourcemap_cache_expiration(self): self.check_rum_error_sourcemap(False, expected_err="No Sourcemap available for") +class SourcemappingDisabledIntegrationTest(ClientSideElasticTest): + config_overrides = { + "rum_sourcemapping_disabled": True, + } + + @unittest.skipUnless(INTEGRATION_TESTS, "integration test") + def test_rum_transaction(self): + path = 'http://localhost:8000/test/e2e/general-usecase/bundle.js.map' + r = self.upload_sourcemap(file_name='bundle.js.map', + bundle_filepath=path, + service_version='1.0.0') + assert r.status_code == 403, r.status_code + + self.load_docs_with_template(self.get_transaction_payload_path(), + self.intake_url, + 'transaction', + 2) + rs = self.es.search(index=self.index_span, params={"rest_total_hits_as_int": "true"}) + assert rs['hits']['total'] == 1, "found {} documents, expected {}".format( + rs['hits']['total'], 1) + frames_checked = 0 + for doc in rs['hits']['hits']: + span = doc["_source"]["span"] + for frame in span["stacktrace"]: + frames_checked += 1 + assert "sourcemap" not in frame, frame + assert frames_checked > 0, "no frames checked" + + class ExpvarDisabledIntegrationTest(ExpvarBaseTest): config_overrides = {"expvar_enabled": "false"}