Merge pull request #211 from solarwindscloud/NH-65068-is-lambda

NH-65068 Add is_lambda checks and todos to APM Python
solarwinds · Nov 7, 2023 · 9ec9afb · 9ec9afb
2 parents 64c0cd5 + dbbd2d9
commit 9ec9afb
Show file tree

Hide file tree

Showing 6 changed files with 183 additions and 77 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased](https://github.com/solarwindscloud/solarwinds-apm-python/compare/rel-0.18.0...HEAD)
 
 ### Changed
+- Change c-lib usage and `agent_enabled` calculation if `is_lambda` ([#211](https://github.com/solarwindscloud/solarwinds-apm-python/pull/211))
 - Updated Makefile for APM Python lambda builds ([#212](https://github.com/solarwindscloud/solarwinds-apm-python/pull/212))
 
 ## [0.18.0](https://github.com/solarwindscloud/solarwinds-apm-python/releases/tag/rel-0.18.0) - 2023-10-31

diff --git a/solarwinds_apm/apm_config.py b/solarwinds_apm/apm_config.py
@@ -20,6 +20,7 @@
 from opentelemetry.sdk.resources import Resource
 from pkg_resources import iter_entry_points
 
+import solarwinds_apm.apm_noop as noop_extension
 from solarwinds_apm import apm_logging
 from solarwinds_apm.apm_constants import (
     INTL_SWO_AO_COLLECTOR,
@@ -115,6 +116,7 @@ def __init__(
             "experimental": {},
             "transaction_name": None,
         }
+        self.is_lambda = self._is_lambda()
         self.agent_enabled = True
         self.update_with_cnf_file()
         self.update_with_env_var()
@@ -132,25 +134,14 @@ def __init__(
             self.service_name,
         )
 
-        if self.agent_enabled:
-            try:
-                # pylint: disable=import-outside-toplevel
-                import solarwinds_apm.extension.oboe as c_extension
-            except ImportError as err:
-                # At this point, if agent_enabled but cannot import
-                # extension then something unexpected happened
-                logger.error(
-                    "Could not import extension. Please contact %s. Tracing disabled: %s",
-                    INTL_SWO_SUPPORT_EMAIL,
-                    err,
-                )
-                # pylint: disable=import-outside-toplevel
-                import solarwinds_apm.apm_noop as c_extension
-        else:
-            # pylint: disable-next=import-outside-toplevel
-            import solarwinds_apm.apm_noop as c_extension
-        self.extension = c_extension
-        self.context = self.extension.Context
+        # Calculate c-lib extension usage
+        # TODO Used returned OboeAPI
+        #      https://swicloud.atlassian.net/browse/NH-64716
+        self.extension, self.context = self._get_extension_components(
+            self.agent_enabled,
+            self.is_lambda,
+        )
+
         self.context.setTracingMode(self.__config["tracing_mode"])
         self.context.setTriggerMode(self.__config["trigger_trace"])
 
@@ -159,13 +150,46 @@ def __init__(
 
         logger.debug("Set ApmConfig as: %s", self)
 
+    def _get_extension_components(
+        self,
+        agent_enabled: bool,
+        is_lambda: bool,
+    ) -> None:
+        """Returns c-lib extension or noop components based on agent_enabled, is_lambda.
+
+        TODO: Return OboeAPI as noop or c-lib version
+        https://swicloud.atlassian.net/browse/NH-64716
+
+        agent_enabled T, is_lambda F -> c-lib extension, c-lib Context
+        agent_enabled T, is_lambda T -> no-op extension, no-op Context
+        agent_enabled F              -> all no-op
+        """
+        if not agent_enabled:
+            return noop_extension, noop_extension.Context
+        try:
+            # pylint: disable=import-outside-toplevel
+            import solarwinds_apm.extension.oboe as c_extension
+        except ImportError as err:
+            # At this point, if agent_enabled but cannot import
+            # extension then something unexpected happened
+            logger.error(
+                "Could not import extension. Please contact %s. Tracing disabled: %s",
+                INTL_SWO_SUPPORT_EMAIL,
+                err,
+            )
+            return noop_extension, noop_extension.Context
+
+        if is_lambda:
+            return noop_extension, noop_extension.Context
+        return c_extension, c_extension.Context
+
     def _is_lambda(self) -> bool:
         """Checks if agent is running in an AWS Lambda environment."""
         if os.environ.get("AWS_LAMBDA_FUNCTION_NAME") and os.environ.get(
             "LAMBDA_TASK_ROOT"
         ):
             logger.warning(
-                "AWS Lambda is not yet supported by Python SolarWinds APM."
+                "AWS Lambda is experimental in Python SolarWinds APM."
             )
             return True
         return False
@@ -185,6 +209,19 @@ def _calculate_agent_enabled_platform(self) -> bool:
             return False
         return True
 
+    def _calculate_agent_enabled_config_lambda(self) -> bool:
+        """Checks if agent is enabled/disabled based on config in lambda environment:
+        - SW_APM_AGENT_ENABLED (optional) (env var or cnf file)
+        """
+        if not self.agent_enabled:
+            logger.info(
+                "SolarWinds APM is disabled and will not report any traces because the environment variable "
+                "SW_APM_AGENT_ENABLED or the config file agentEnabled field is set to 'false'! If this is not intended either unset the variable or set it to "
+                "a value other than false. Note that SW_APM_AGENT_ENABLED/agentEnabled is case-insensitive."
+            )
+            return False
+        return True
+
     # TODO: Account for in-code config with kwargs after alpha
     # pylint: disable=too-many-branches,too-many-return-statements
     def _calculate_agent_enabled_config(self) -> bool:
@@ -194,8 +231,11 @@ def _calculate_agent_enabled_config(self) -> bool:
         - OTEL_PROPAGATORS     (optional) (env var only)
         - OTEL_TRACES_EXPORTER (optional) (env var only)
         """
+        if self.is_lambda:
+            return self._calculate_agent_enabled_config_lambda()
+
         # (1) SW_APM_SERVICE_KEY
-        if not self.__config.get("service_key") and not self._is_lambda():
+        if not self.__config.get("service_key"):
             logger.error("Missing service key. Tracing disabled.")
             return False
         # Key must be at least one char + ":" + at least one other char
@@ -267,14 +307,16 @@ def _calculate_agent_enabled_config(self) -> bool:
             return False
 
         # (4) OTEL_TRACES_EXPORTER
+        # TODO Relax traces exporter requirements outside lambda
+        #      https://swicloud.atlassian.net/browse/NH-65713
         try:
             # SolarWindsDistro._configure does setdefault so this shouldn't
             # be None, but safer and more explicit this way
             environ_exporters = os.environ.get(
                 OTEL_TRACES_EXPORTER,
                 INTL_SWO_DEFAULT_TRACES_EXPORTER,
             ).split(",")
-            # If not using the default propagators,
+            # If not using the default exporters,
             # can any arbitrary list BUT
             # (1) must include solarwinds_exporter
             # (2) other exporters must be loadable by OTel
@@ -530,7 +572,6 @@ def _snake_to_camel_case(key):
             self.agent_enabled = cnf_agent_enabled
 
         available_cnf = set(self.__config.keys())
-        # TODO after alpha: is_lambda
         for key in available_cnf:
             # Use internal snake_case config keys to check JSON config file camelCase keys
             val = cnf_dict.get(_snake_to_camel_case(key))
@@ -611,11 +652,12 @@ def update_with_env_var(self) -> None:
             self.agent_enabled = env_agent_enabled
 
         available_envvs = set(self.__config.keys())
-        # TODO after alpha: is_lambda
         for key in available_envvs:
             if key == "transaction":
                 # we do not allow complex config options to be set via environment variables
                 continue
+            # TODO Add experimental trace flag, clean up
+            #      https://swicloud.atlassian.net/browse/NH-65067
             if key == "experimental":
                 # but we do allow flat SW_APM_EXPERIMENTAL_OTEL_COLLECTOR setting to match js
                 key = self._EXP_PREFIX + "otel_collector"
@@ -700,7 +742,6 @@ def _set_config_value(self, keys_str: str, val: Any) -> Any:
                 )
                 self.__config[key] = oboe_trigger_trace
             elif keys == ["reporter"]:
-                # TODO: support 'lambda'
                 if not isinstance(val, str) or val.lower() not in (
                     "udp",
                     "ssl",
@@ -716,6 +757,8 @@ def _set_config_value(self, keys_str: str, val: Any) -> Any:
                 self.__config[key] = val
                 # update logging level of agent logger
                 apm_logging.set_sw_log_level(val)
+            # TODO Add experimental trace flag, clean up
+            #      https://swicloud.atlassian.net/browse/NH-65067
             elif keys == ["experimental"]:
                 for exp_k, exp_v in val.items():
                     if exp_k in self._EXP_KEYS:
@@ -727,6 +770,8 @@ def _set_config_value(self, keys_str: str, val: Any) -> Any:
                             )
                         else:
                             self.__config["experimental"][exp_k] = exp_v
+            # TODO Add experimental trace flag, clean up
+            #      https://swicloud.atlassian.net/browse/NH-65067
             elif keys == ["experimental_otel_collector"]:
                 val = self.convert_to_bool(val)
                 if val is None:

diff --git a/solarwinds_apm/configurator.py b/solarwinds_apm/configurator.py
@@ -11,6 +11,7 @@
 import os
 import sys
 import time
+from typing import TYPE_CHECKING
 
 from opentelemetry import metrics, trace
 from opentelemetry.environment_variables import (
@@ -46,7 +47,6 @@
 )
 from solarwinds_apm.apm_fwkv_manager import SolarWindsFrameworkKvManager
 from solarwinds_apm.apm_meter_manager import SolarWindsMeterManager
-from solarwinds_apm.apm_noop import Reporter
 from solarwinds_apm.apm_noop import SolarWindsMeterManager as NoopMeterManager
 from solarwinds_apm.apm_oboe_codes import OboeReporterCode
 from solarwinds_apm.apm_txname_manager import SolarWindsTxnNameManager
@@ -61,6 +61,9 @@
 )
 from solarwinds_apm.version import __version__
 
+if TYPE_CHECKING:
+    from solarwinds_apm.extension.oboe import Reporter
+
 solarwinds_apm_logger = apm_logging.logger
 logger = logging.getLogger(__name__)
 
@@ -80,6 +83,8 @@ def _configure(self, **kwargs: int) -> None:
         apm_fwkv_manager = SolarWindsFrameworkKvManager()
         apm_config = SolarWindsApmConfig()
 
+        # TODO Add experimental trace flag, clean up
+        #      https://swicloud.atlassian.net/browse/NH-65067
         if not apm_config.get("experimental").get("otel_collector") is True:
             logger.debug(
                 "Experimental otel_collector flag not configured. Creating meter manager as no-op."
@@ -110,7 +115,7 @@ def _configure_otel_components(
         """Configure OTel sampler, exporter, propagator, response propagator"""
         self._configure_sampler(apm_config)
         if apm_config.agent_enabled:
-            self._configure_metrics_span_processor(
+            self._configure_inbound_metrics_span_processor(
                 apm_txname_manager,
                 apm_config,
             )
@@ -165,12 +170,16 @@ def _configure_sampler(
             ),
         )
 
-    def _configure_metrics_span_processor(
+    def _configure_inbound_metrics_span_processor(
         self,
         apm_txname_manager: SolarWindsTxnNameManager,
         apm_config: SolarWindsApmConfig,
     ) -> None:
-        """Configure SolarWindsInboundMetricsSpanProcessor"""
+        """Configure SolarWindsInboundMetricsSpanProcessor.
+        Note: if config's extension is no-op, the processor will run
+        but will not export inbound metrics."""
+        # TODO Refactor span processors
+        #      https://swicloud.atlassian.net/browse/NH-65061
         trace.get_tracer_provider().add_span_processor(
             SolarWindsInboundMetricsSpanProcessor(
                 apm_txname_manager,
@@ -184,7 +193,11 @@ def _configure_otlp_metrics_span_processor(
         apm_config: SolarWindsApmConfig,
         apm_meters: SolarWindsMeterManager,
     ) -> None:
-        """Configure SolarWindsOTLPMetricsSpanProcessor"""
+        """Configure SolarWindsOTLPMetricsSpanProcessor.
+        If no meters/instruments are initialized, the processor will
+        run but will not collect/flush OTLP metrics."""
+        # TODO Add experimental trace flag, clean up
+        #      https://swicloud.atlassian.net/browse/NH-65067
         if not apm_config.get("experimental").get("otel_collector") is True:
             logger.debug(
                 "Experimental otel_collector flag not configured. Not configuring OTLP metrics span processor."
@@ -207,8 +220,10 @@ def _configure_exporter(
         apm_config: SolarWindsApmConfig,
     ) -> None:
         """Configure SolarWinds OTel span exporters, defaults or environment
-        configured, or none if agent disabled. Initialization of SolarWinds
-        exporter requires a liboboe reporter and agent_enabled flag."""
+        configured, or none if agent disabled.
+
+        Initialization of SolarWinds exporter requires a liboboe reporter
+        Note: if reporter is no-op, the SW exporter will not export spans."""
         if not apm_config.agent_enabled:
             logger.error("Tracing disabled. Cannot set span_processor.")
             return
@@ -370,7 +385,10 @@ def _initialize_solarwinds_reporter(
         self,
         apm_config: SolarWindsApmConfig,
     ) -> "Reporter":
-        """Initialize SolarWinds reporter used by sampler and exporter, using SolarWindsApmConfig. This establishes collector and sampling settings in a background thread."""
+        """Initialize SolarWinds reporter used by sampler and exporter, using SolarWindsApmConfig.
+        This establishes collector and sampling settings in a background thread.
+
+        Note: if config's extension is no-op, this has no effect."""
         reporter_kwargs = {
             "hostname_alias": apm_config.get("hostname_alias"),
             "log_level": apm_config.get("debug_level"),

diff --git a/solarwinds_apm/inbound_metrics_processor.py b/solarwinds_apm/inbound_metrics_processor.py
@@ -26,6 +26,9 @@
 
 
 class SolarWindsInboundMetricsSpanProcessor(SpanProcessor):
+    # TODO Refactor span processors
+    #      https://swicloud.atlassian.net/browse/NH-65061
+
     _TRANSACTION_NAME = "transaction_name"
     _HTTP_METHOD = SpanAttributes.HTTP_METHOD  # "http.method"
     _HTTP_ROUTE = SpanAttributes.HTTP_ROUTE  # "http.route"