From 5d7ba8b839df75cee2d9b4996c8ff9bfdee70df8 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Thu, 8 Feb 2024 11:17:37 -0800 Subject: [PATCH 1/4] Split _calculate_service_name; do not update_service_key if lambda --- solarwinds_apm/apm_config.py | 62 ++++++++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 6 deletions(-) diff --git a/solarwinds_apm/apm_config.py b/solarwinds_apm/apm_config.py index 9393f8c5..8ea4d017 100644 --- a/solarwinds_apm/apm_config.py +++ b/solarwinds_apm/apm_config.py @@ -130,11 +130,12 @@ def __init__( self.agent_enabled, otel_resource, ) - self.__config["service_key"] = self._update_service_key_name( - self.agent_enabled, - self.__config["service_key"], - self.service_name, - ) + if not self.is_lambda: + self.__config["service_key"] = self._update_service_key_name( + self.agent_enabled, + self.__config["service_key"], + self.service_name, + ) # Update and apply logging settings to Python logger self.update_log_settings() @@ -415,7 +416,39 @@ def _calculate_agent_enabled(self) -> bool: logger.debug("agent_enabled: %s", agent_enabled) return agent_enabled - def _calculate_service_name( + def _calculate_service_name_lambda( + self, + agent_enabled: bool, + otel_resource: "Resource", + ) -> str: + """Calculate `service.name` by priority system (decreasing): + 1. OTEL_SERVICE_NAME + 2. AWS_LAMBDA_FUNCTION_NAME + + Note: 1 is always set by the current otel-instrument lambda exec wrapper + if used. The wrapper also sets service_name as the function_name, if + former is not provided. + + If otel-instrument did not do the above, the passed in OTel Resource + likely has a `service.name` already calculated by merging OTEL_SERVICE_NAME + / OTEL_RESOURCE_ATTRIBUTES with defaults. + + We assume 2 is not none/empty because is_lambda check by caller should + be True. + """ + otel_service_name = otel_resource.attributes.get("service.name", None) + + if not otel_service_name: + return self.lambda_function_name + + if otel_service_name == "unknown_service": + # OTEL_SERVICE_NAME was not set + # and otel-instrument did not wrap instrumentation + return self.lambda_function_name + + return otel_service_name + + def _calculate_service_name_apm_proto( self, agent_enabled: bool, otel_resource: "Resource", @@ -455,6 +488,23 @@ def _calculate_service_name( service_name = otel_service_name return service_name + def _calculate_service_name( + self, + agent_enabled: bool, + otel_resource: "Resource", + ) -> str: + """Calculate service_name""" + if self.is_lambda: + return self._calculate_service_name_lambda( + agent_enabled, + otel_resource, + ) + + return self._calculate_service_name_apm_proto( + agent_enabled, + otel_resource, + ) + def _update_service_key_name( self, agent_enabled: bool, From 5cea00ccdabb5cbd8175c3c83d927d8482e53272 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Thu, 8 Feb 2024 14:02:47 -0800 Subject: [PATCH 2/4] Testrelease 1.2.1.1 --- solarwinds_apm/version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solarwinds_apm/version.py b/solarwinds_apm/version.py index c68196d1..bd460b73 100644 --- a/solarwinds_apm/version.py +++ b/solarwinds_apm/version.py @@ -1 +1 @@ -__version__ = "1.2.0" +__version__ = "1.2.1.1" From 035b533a23f7fbf5b870eeb2e99042ff31275293 Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Thu, 8 Feb 2024 16:17:56 -0800 Subject: [PATCH 3/4] calculate_service_name_lambda does not consider agent_enabled --- solarwinds_apm/apm_config.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/solarwinds_apm/apm_config.py b/solarwinds_apm/apm_config.py index 8ea4d017..fc2de853 100644 --- a/solarwinds_apm/apm_config.py +++ b/solarwinds_apm/apm_config.py @@ -418,7 +418,6 @@ def _calculate_agent_enabled(self) -> bool: def _calculate_service_name_lambda( self, - agent_enabled: bool, otel_resource: "Resource", ) -> str: """Calculate `service.name` by priority system (decreasing): @@ -496,7 +495,6 @@ def _calculate_service_name( """Calculate service_name""" if self.is_lambda: return self._calculate_service_name_lambda( - agent_enabled, otel_resource, ) From b6bb5e6e5bf6b4af83131f6a8bd5f5e6851cbd8b Mon Sep 17 00:00:00 2001 From: tammy-baylis-swi Date: Thu, 8 Feb 2024 16:33:08 -0800 Subject: [PATCH 4/4] Add and update tests --- tests/unit/test_apm_config/test_apm_config.py | 51 ------ .../test_apm_config_service_name.py | 166 ++++++++++++++++++ 2 files changed, 166 insertions(+), 51 deletions(-) create mode 100644 tests/unit/test_apm_config/test_apm_config_service_name.py diff --git a/tests/unit/test_apm_config/test_apm_config.py b/tests/unit/test_apm_config/test_apm_config.py index 258feae7..86272863 100644 --- a/tests/unit/test_apm_config/test_apm_config.py +++ b/tests/unit/test_apm_config/test_apm_config.py @@ -679,57 +679,6 @@ def test_set_config_value_default_log_type( if old_log_type: os.environ["SW_APM_LOG_TYPE"] = old_log_type - def test__calculate_service_name_agent_disabled(self): - test_config = apm_config.SolarWindsApmConfig() - result = test_config._calculate_service_name( - False, - {} - ) - assert result == "" - - def test__calculate_service_name_no_otel_service_name( - self, - mocker, - ): - mocker.patch.dict(os.environ, { - "SW_APM_SERVICE_KEY": "service_key_with:sw_service_name", - }) - test_config = apm_config.SolarWindsApmConfig() - result = test_config._calculate_service_name( - True, - Resource.create({"service.name": None}) - ) - assert result == "sw_service_name" - - def test__calculate_service_name_default_unknown_otel_service_name( - self, - mocker, - ): - mocker.patch.dict(os.environ, { - "SW_APM_SERVICE_KEY": "service_key_with:sw_service_name", - }) - test_config = apm_config.SolarWindsApmConfig() - result = test_config._calculate_service_name( - True, - # default is unknown_service - Resource.create() - ) - assert result == "sw_service_name" - - def test__calculate_service_name_use_otel_service_name( - self, - mocker, - ): - mocker.patch.dict(os.environ, { - "SW_APM_SERVICE_KEY": "service_key_with:sw_service_name", - }) - test_config = apm_config.SolarWindsApmConfig() - result = test_config._calculate_service_name( - True, - Resource.create({"service.name": "foobar"}) - ) - assert result == "foobar" - def test__update_service_key_name_not_agent_enabled(self): test_config = apm_config.SolarWindsApmConfig() result = test_config._update_service_key_name( diff --git a/tests/unit/test_apm_config/test_apm_config_service_name.py b/tests/unit/test_apm_config/test_apm_config_service_name.py new file mode 100644 index 00000000..e09cee4b --- /dev/null +++ b/tests/unit/test_apm_config/test_apm_config_service_name.py @@ -0,0 +1,166 @@ +# © 2024 SolarWinds Worldwide, LLC. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at:http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + +import os + +from opentelemetry.sdk.resources import Resource + +from solarwinds_apm import apm_config + + +class TestSolarWindsApmConfigServiceName: + def test__calculate_service_name_is_lambda(self, mocker): + mock_calc_proto = mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig._calculate_service_name_apm_proto" + ) + mock_calc_lambda = mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig._calculate_service_name_lambda" + ) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.calculate_is_lambda", + return_value=True, + ) + test_config = apm_config.SolarWindsApmConfig("foo") + test_config._calculate_service_name( + True, + {}, + ) + mock_calc_proto.assert_not_called() + # called twice because of init, and we call again + mock_calc_lambda.assert_has_calls( + [ + mocker.call("foo"), + mocker.call({}), + ] + ) + + def test__calculate_service_name_not_is_lambda(self, mocker): + mock_calc_proto = mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig._calculate_service_name_apm_proto" + ) + mock_calc_lambda = mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig._calculate_service_name_lambda" + ) + mocker.patch( + "solarwinds_apm.apm_config.SolarWindsApmConfig.calculate_is_lambda", + return_value=False, + ) + test_config = apm_config.SolarWindsApmConfig("foo") + test_config._calculate_service_name( + True, + {}, + ) + # called twice because of init, and we call again + mock_calc_proto.assert_has_calls( + [ + mocker.call(False, "foo"), + mocker.call(True, {}), + ] + ) + mock_calc_lambda.assert_not_called() + + +class TestSolarWindsApmConfigServiceNameApmProto: + def test__calculate_service_name_apm_proto_agent_disabled(self): + test_config = apm_config.SolarWindsApmConfig() + result = test_config._calculate_service_name_apm_proto( + False, + {} + ) + assert result == "" + + def test__calculate_service_name_apm_proto_no_otel_service_name( + self, + mocker, + ): + mocker.patch.dict(os.environ, { + "SW_APM_SERVICE_KEY": "service_key_with:sw_service_name", + }) + test_config = apm_config.SolarWindsApmConfig() + result = test_config._calculate_service_name_apm_proto( + True, + Resource.create({"service.name": None}) + ) + assert result == "sw_service_name" + + def test__calculate_service_name_apm_proto_default_unknown_otel_service_name( + self, + mocker, + ): + mocker.patch.dict(os.environ, { + "SW_APM_SERVICE_KEY": "service_key_with:sw_service_name", + }) + test_config = apm_config.SolarWindsApmConfig() + result = test_config._calculate_service_name_apm_proto( + True, + # default is unknown_service + Resource.create() + ) + assert result == "sw_service_name" + + def test__calculate_service_name_apm_proto_use_otel_service_name( + self, + mocker, + ): + mocker.patch.dict(os.environ, { + "SW_APM_SERVICE_KEY": "service_key_with:sw_service_name", + }) + test_config = apm_config.SolarWindsApmConfig() + result = test_config._calculate_service_name_apm_proto( + True, + Resource.create({"service.name": "foobar"}) + ) + assert result == "foobar" + +class TestSolarWindsApmConfigServiceNameLambda: + def test__calculate_service_name_lambda_no_otel_name( + self, + mocker, + ): + mocker.patch.dict(os.environ, { + "AWS_LAMBDA_FUNCTION_NAME": "foo-fn", + }) + test_config = apm_config.SolarWindsApmConfig() + result = test_config._calculate_service_name_lambda( + Resource.create({}) + ) + assert result == "foo-fn" + + def test__calculate_service_name_lambda_empty_otel_name( + self, + mocker, + ): + mocker.patch.dict(os.environ, { + "AWS_LAMBDA_FUNCTION_NAME": "foo-fn", + }) + test_config = apm_config.SolarWindsApmConfig() + result = test_config._calculate_service_name_lambda( + Resource.create({"service.name": ""}) + ) + assert result == "foo-fn" + + def test__calculate_service_name_lambda_otel_name_unknown( + self, + mocker, + ): + mocker.patch.dict(os.environ, { + "AWS_LAMBDA_FUNCTION_NAME": "foo-fn", + }) + test_config = apm_config.SolarWindsApmConfig() + result = test_config._calculate_service_name_lambda( + Resource.create({"service.name": "unknown_service"}) + ) + assert result == "foo-fn" + + def test__calculate_service_name_lambda_otel_name_ok( + self, + mocker, + ): + test_config = apm_config.SolarWindsApmConfig() + result = test_config._calculate_service_name_lambda( + Resource.create({"service.name": "foo-service-name"}) + ) + assert result == "foo-service-name"