Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NH-68264 Upgrade c-lib 14 #259

Merged
merged 18 commits into from
Jan 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 52 additions & 12 deletions solarwinds_apm/apm_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,9 @@ def __init__(
),
"collector": "", # the collector address in host:port format.
"reporter": "", # the reporter mode, either 'udp' or 'ssl'.
"log_type": apm_logging.ApmLoggingType.default_type(),
"debug_level": apm_logging.ApmLoggingLevel.default_level(),
"logname": "",
"service_key": "",
"hostname_alias": "",
"trustedpath": "",
Expand All @@ -104,7 +106,6 @@ def __init__(
"ec2_metadata_timeout": 1000,
"max_flush_wait_time": -1,
"max_transactions": -1,
"logname": "",
"trace_metrics": -1,
"token_bucket_capacity": -1,
"token_bucket_rate": -1,
Expand Down Expand Up @@ -134,17 +135,29 @@ def __init__(
self.__config["service_key"],
self.service_name,
)
self.update_log_type()
# update logging level of Python logging logger
apm_logging.set_sw_log_level(self.__config["debug_level"])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved this up here instead of in _set_config_value for clarity and so only called once.


# Calculate c-lib extension usage
(
self.extension,
self.context,
self.oboe_api,
oboe_api_swig,
oboe_api_options_swig,
) = self._get_extension_components(
self.agent_enabled,
self.is_lambda,
)

# Create OboeAPI options using extension and __config
oboe_api_options = oboe_api_options_swig()
oboe_api_options.logging_options.level = self.__config["debug_level"]
oboe_api_options.logging_options.type = self.__config["log_type"]
self.oboe_api = oboe_api_swig(
oboe_api_options,
)

self.context.setTracingMode(self.__config["tracing_mode"])
self.context.setTriggerMode(self.__config["trigger_trace"])

Expand All @@ -160,15 +173,16 @@ def _get_extension_components(
) -> None:
"""Returns c-lib extension or noop components based on agent_enabled, is_lambda.

agent_enabled T, is_lambda F -> c-lib extension, c-lib Context, no-op settings API
agent_enabled T, is_lambda T -> no-op extension, no-op Context, c-lib settings API
agent_enabled T, is_lambda F -> c-lib extension, c-lib Context, no-op settings API, no-op API options
agent_enabled T, is_lambda T -> no-op extension, no-op Context, c-lib settings API, c-lib API options
agent_enabled F -> all no-op
"""
if not agent_enabled:
return (
noop_extension,
noop_extension.Context,
noop_extension.OboeAPI,
noop_extension.OboeAPIOptions,
)

try:
Expand All @@ -186,28 +200,41 @@ def _get_extension_components(
noop_extension,
noop_extension.Context,
noop_extension.OboeAPI,
noop_extension.OboeAPIOptions,
)

if is_lambda:
try:
# pylint: disable=import-outside-toplevel,no-name-in-module
from solarwinds_apm.extension.oboe import OboeAPI as oboe_api
from solarwinds_apm.extension.oboe import (
OboeAPIOptions as api_options,
)
except ImportError as err:
# c-lib version may not have settings API
# TODO Update this message to contact support after c-lib 14
# https://swicloud.atlassian.net/browse/NH-64716
logger.warning(
"Could not import API in lambda mode. Installed layer version not compatible. Tracing disabled: %s",
"Could not import API in lambda mode. Please contact %s. Tracing disabled: %s",
INTL_SWO_SUPPORT_EMAIL,
err,
)
return (
noop_extension,
noop_extension.Context,
noop_extension.OboeAPI,
noop_extension.OboeAPIOptions,
)
return noop_extension, noop_extension.Context, oboe_api
return (
noop_extension,
noop_extension.Context,
oboe_api,
api_options,
)

return c_extension, c_extension.Context, noop_extension.OboeAPI
return (
c_extension,
c_extension.Context,
noop_extension.OboeAPI,
noop_extension.OboeAPIOptions,
)

@classmethod
def calculate_is_lambda(cls) -> bool:
Expand Down Expand Up @@ -436,6 +463,21 @@ def _update_service_key_name(
# Else no need to update service_key when not reporting
return service_key

def update_log_type(
self,
) -> None:
"""Updates agent log type depending on other configs.

SW_APM_DEBUG_LEVEL -1 will set c-lib log_type to disabled (4).
SW_APM_LOGNAME not None will set c-lib log_type to file (2).
"""
if self.__config["debug_level"] == -1:
self.__config[
"log_type"
] = apm_logging.ApmLoggingType.disabled_type()
elif self.__config["logname"]:
self.__config["log_type"] = apm_logging.ApmLoggingType.file_type()

def _calculate_metric_format(self) -> int:
"""Return one of 1 (TransactionResponseTime only) or 2 (default; ResponseTime only). Note: 0 (both) is no longer supported. Based on collector URL which may have a port e.g. foo-collector.com:443"""
metric_format = 2
Expand Down Expand Up @@ -777,8 +819,6 @@ def _set_config_value(self, keys_str: str, val: Any) -> Any:
if not apm_logging.ApmLoggingLevel.is_valid_level(val):
raise ValueError
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"]:
Expand Down
41 changes: 38 additions & 3 deletions solarwinds_apm/apm_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,45 @@
import os


class ApmLoggingType:
"""Mapping of supported solarwinds_apm library log types"""

log_types = {
"STDERR": 0,
"FILE": 2,
"DISABLED": 4,
}
Comment on lines +44 to +48
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've not included stdout nor null because there currently are no cases where APM Python would need to set them. But if too strict I can add them.


@classmethod
def default_type(cls):
"""Returns integer representation of default log type"""
return cls.log_types["STDERR"]

@classmethod
def disabled_type(cls):
"""Returns integer representation of disabled log type"""
return cls.log_types["DISABLED"]

@classmethod
def file_type(cls):
"""Returns integer representation of to-file log type"""
return cls.log_types["FILE"]

@classmethod
def is_valid_log_type(cls, log_type):
"""Returns True if the provided type is a valid interger representation of log type, False otherwise."""
try:
log_type = int(log_type)
return bool(log_type in list(cls.log_types.values()))
except (ValueError, TypeError):
return cls.default_type()


class ApmLoggingLevel:
"""Abstract mapping class providing a conversion between solarwinds_apm agent logging level and Python logging module
"""Mapping class providing a conversion between solarwinds_apm library logging level and Python logging module
logging levels.
The solarwinds_apm package has seven different log levels, which are defined in the debug_levels dictionary.
The solarwinds_apm package has six main log levels, which are defined in the debug_levels dictionary.
There is also a special 7th OBOE_DEBUG_DISABLE for disabling logging, as there is no "disabled" level in Python logging nor agent logging.
"""

# maps string representation of solarwinds_apm.sw_logging levels to their integer counterpart
Expand Down Expand Up @@ -75,7 +110,7 @@ def default_level(cls):

@classmethod
def is_valid_level(cls, level):
"""Returns True if the provided level is a valid interger representation of a solarwinds_apm.sw_logging level,
"""Returns True if the provided level is a valid integer representation of a solarwinds_apm.sw_logging level,
False otherwise."""
try:
level = int(level)
Expand Down
9 changes: 1 addition & 8 deletions solarwinds_apm/apm_meter_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,7 @@

if TYPE_CHECKING:
from solarwinds_apm.apm_config import SolarWindsApmConfig

try:
# c-lib <14 does not have OboeAPI
# TODO remove the except after upgrading
# https://swicloud.atlassian.net/browse/NH-68264
from solarwinds_apm.extension.oboe import OboeAPI
except ImportError:
from solarwinds_apm.apm_noop import OboeAPI
from solarwinds_apm.extension.oboe import OboeAPI

logger = logging.getLogger(__name__)

Expand Down
11 changes: 11 additions & 0 deletions solarwinds_apm/apm_noop.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,17 @@ def __init__(self, *args, **kwargs):
)


class LoggingOptions:
def __init__(self, *args, **kwargs):
self.level = 0
self.type = 0


class OboeAPIOptions:
def __init__(self, *args, **kwargs):
self.logging_options = LoggingOptions()


class OboeAPI:
def __init__(self, *args, **kwargs):
pass
Expand Down
16 changes: 3 additions & 13 deletions solarwinds_apm/configurator.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,7 @@
from solarwinds_apm.version import __version__

if TYPE_CHECKING:
from solarwinds_apm.extension.oboe import Reporter

try:
# c-lib <14 does not have OboeAPI
# TODO remove the except after upgrading
# https://swicloud.atlassian.net/browse/NH-68264
from solarwinds_apm.extension.oboe import OboeAPI
except ImportError:
from solarwinds_apm.apm_noop import OboeAPI
from solarwinds_apm.extension.oboe import OboeAPI, Reporter

solarwinds_apm_logger = apm_logging.logger
logger = logging.getLogger(__name__)
Expand All @@ -95,10 +87,7 @@ def _configure(self, **kwargs: int) -> None:
apm_txname_manager = SolarWindsTxnNameManager()
apm_fwkv_manager = SolarWindsFrameworkKvManager()
apm_config = SolarWindsApmConfig()
# TODO add args
# https://swicloud.atlassian.net/browse/NH-68264
# See also SolarWindsApmConfig._get_extension_components
oboe_api = apm_config.oboe_api()
oboe_api = apm_config.oboe_api

# TODO Add experimental trace flag, clean up
# https://swicloud.atlassian.net/browse/NH-65067
Expand Down Expand Up @@ -474,6 +463,7 @@ def _initialize_solarwinds_reporter(
Note: if config's extension is no-op, this has no effect."""
reporter_kwargs = {
"hostname_alias": apm_config.get("hostname_alias"),
"log_type": apm_config.get("log_type"),
"log_level": apm_config.get("debug_level"),
"log_file_path": apm_config.get("logname"),
"max_transactions": apm_config.get("max_transactions"),
Expand Down
2 changes: 1 addition & 1 deletion solarwinds_apm/extension/VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
13.0.0
14.0.0
9 changes: 1 addition & 8 deletions solarwinds_apm/sampler.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,7 @@

if TYPE_CHECKING:
from solarwinds_apm.apm_config import SolarWindsApmConfig

try:
# c-lib <14 does not have OboeAPI
# TODO remove the except after upgrading
# https://swicloud.atlassian.net/browse/NH-68264
from solarwinds_apm.extension.oboe import OboeAPI
except ImportError:
from solarwinds_apm.apm_noop import OboeAPI
from solarwinds_apm.extension.oboe import OboeAPI

logger = logging.getLogger(__name__)

Expand Down
8 changes: 1 addition & 7 deletions tests/integration/test_base_sw_headers_attrs.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,10 @@
from solarwinds_apm.apm_config import SolarWindsApmConfig
from solarwinds_apm.configurator import SolarWindsConfigurator
from solarwinds_apm.distro import SolarWindsDistro
from solarwinds_apm.extension.oboe import OboeAPI
from solarwinds_apm.propagator import SolarWindsPropagator
from solarwinds_apm.sampler import ParentBasedSwSampler

try:
# c-lib <14 does not have OboeAPI
# TODO remove the except after upgrading
# https://swicloud.atlassian.net/browse/NH-68264
from solarwinds_apm.extension.oboe import OboeAPI
except ImportError:
from solarwinds_apm.apm_noop import OboeAPI

class TestBaseSwHeadersAndAttributes(TestBase):
"""
Expand Down
8 changes: 4 additions & 4 deletions tests/unit/test_apm_config/fixtures/cnf_dict.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ def fixture_cnf_dict():
"collector": "foo-bar",
"reporter": "udp",
"debugLevel": 6,
"logname": "foo-bar",
"serviceKey": "not-good-to-put-here:still-could-be-used",
"hostnameAlias": "foo-bar",
"trustedpath": "foo-bar",
"eventsFlushInterval": 2,
"maxRequestSizeBytes": 2,
"ec2MetadataTimeout": 1234,
"maxFlushWaitTime": 2,
"maxTransactions": 2,
"logname": "foo-bar",
"maxTransactions": 2,
"traceMetrics": 2,
"tokenBucketCapacity": 2,
"tokenBucketRate": 2,
Expand All @@ -37,6 +37,7 @@ def fixture_cnf_dict_enabled_false():
"collector": "foo-bar",
"reporter": "udp",
"debugLevel": 6,
"logname": "foo-bar",
"serviceKey": "not-good-to-put-here:still-could-be-used",
"hostnameAlias": "foo-bar",
"trustedpath": "foo-bar",
Expand All @@ -45,7 +46,6 @@ def fixture_cnf_dict_enabled_false():
"ec2MetadataTimeout": 1234,
"maxFlushWaitTime": 2,
"maxTransactions": 2,
"logname": "foo-bar",
"traceMetrics": 2,
"tokenBucketCapacity": 2,
"tokenBucketRate": 2,
Expand All @@ -65,6 +65,7 @@ def fixture_cnf_dict_enabled_false_mixed_case():
"collector": "foo-bar",
"reporter": "udp",
"debugLevel": 6,
"logname": "foo-bar",
"serviceKey": "not-good-to-put-here:still-could-be-used",
"hostnameAlias": "foo-bar",
"trustedpath": "foo-bar",
Expand All @@ -73,7 +74,6 @@ def fixture_cnf_dict_enabled_false_mixed_case():
"ec2MetadataTimeout": 1234,
"maxFlushWaitTime": 2,
"maxTransactions": 2,
"logname": "foo-bar",
"traceMetrics": 2,
"tokenBucketCapacity": 2,
"tokenBucketRate": 2,
Expand Down
Loading