diff --git a/solarwinds_apm/apm_logging.py b/solarwinds_apm/apm_logging.py index 9f6799b01..2c4f7c520 100644 --- a/solarwinds_apm/apm_logging.py +++ b/solarwinds_apm/apm_logging.py @@ -148,7 +148,13 @@ def _get_logger() -> logging.Logger: ) _logger.warning("") else: - log_level = int(envv_val) + try: + log_level = int(envv_val) + except (ValueError, TypeError) as exc: + _logger.warning( + "Failed to parse SW_APM_DEBUG_LEVEL, using default: %s", + exc, + ) _logger.setLevel(ApmLoggingLevel.logging_map[log_level]) diff --git a/solarwinds_apm/distro.py b/solarwinds_apm/distro.py index 342b88073..39fe4781d 100644 --- a/solarwinds_apm/distro.py +++ b/solarwinds_apm/distro.py @@ -122,7 +122,11 @@ def _get_token_from_service_key(self) -> str | None: logger.debug("Missing service key") return None # Key must be at least one char + ":" + at least one other char - key_parts = [p for p in service_key.split(":") if len(p) > 0] + try: + key_parts = [p for p in service_key.split(":") if len(p) > 0] + except AttributeError: + logger.debug("Invalid service key format") + return None if len(key_parts) != 2: logger.debug("Incorrect service key format") return None diff --git a/solarwinds_apm/oboe/http_sampler.py b/solarwinds_apm/oboe/http_sampler.py index d8fa43f98..df5a14bc7 100644 --- a/solarwinds_apm/oboe/http_sampler.py +++ b/solarwinds_apm/oboe/http_sampler.py @@ -65,7 +65,13 @@ def __init__( self._url = f"https://{self._url}" self._service = config.service self._headers = config.headers - self._hostname = socket.gethostname() + try: + self._hostname = socket.gethostname() + except OSError as exc: + logger.warning( + "Failed to get hostname, using 'localhost': %s", exc + ) + self._hostname = "localhost" self._last_warning_message = None self._shutdown_event = threading.Event() self._daemon_thread = threading.Thread( @@ -152,4 +158,10 @@ def _fetch_from_collector(self): detach(token) response.raise_for_status() logger.debug("received sampling settings response %s", response.text) - return response.json() + try: + return response.json() + except ValueError as exc: + logger.warning( + "Failed to parse JSON response from sampling settings: %s", exc + ) + return {} diff --git a/solarwinds_apm/oboe/oboe_sampler.py b/solarwinds_apm/oboe/oboe_sampler.py index c123cc3c3..0f1b3f913 100644 --- a/solarwinds_apm/oboe/oboe_sampler.py +++ b/solarwinds_apm/oboe/oboe_sampler.py @@ -449,7 +449,14 @@ def parent_based_algo( if s.settings and s.settings.flags & Flags.SAMPLE_THROUGH_ALWAYS: logger.debug("SAMPLE_THROUGH_ALWAYS is set; parent-based sampling") - flags = int(s.trace_state[-2:], 16) + try: + flags = int(s.trace_state[-2:], 16) + except (ValueError, IndexError, TypeError) as exc: + logger.warning( + "Failed to parse trace flags from trace_state; record only: %s", + exc, + ) + return Decision.RECORD_ONLY if flags & TraceFlags.SAMPLED: logger.debug("parent is sampled; record and sample") self.counters.trace_count.add(1, {}, parent_context) diff --git a/solarwinds_apm/oboe/sampler.py b/solarwinds_apm/oboe/sampler.py index 82e8ce41b..25aa8f213 100644 --- a/solarwinds_apm/oboe/sampler.py +++ b/solarwinds_apm/oboe/sampler.py @@ -70,11 +70,14 @@ def http_span_metadata(kind: SpanKind, attributes: Attributes): method = str( attributes.get(HTTP_METHOD, attributes.get(HTTP_REQUEST_METHOD, "")) ) - status = int( - attributes.get( - HTTP_RESPONSE_STATUS_CODE, attributes.get(HTTP_STATUS_CODE, 0) + try: + status = int( + attributes.get( + HTTP_RESPONSE_STATUS_CODE, attributes.get(HTTP_STATUS_CODE, 0) + ) ) - ) + except (ValueError, TypeError): + status = 0 scheme = str( attributes.get(URL_SCHEME, attributes.get(HTTP_SCHEME, "http")) ) diff --git a/solarwinds_apm/oboe/trace_options.py b/solarwinds_apm/oboe/trace_options.py index 9af24fe7f..461a7c670 100644 --- a/solarwinds_apm/oboe/trace_options.py +++ b/solarwinds_apm/oboe/trace_options.py @@ -376,9 +376,15 @@ def validate_signature(header, signature, key, timestamp): return Auth.NO_SIGNATURE_KEY if timestamp is None or abs(int(time.time()) - timestamp) > 5 * 60: return Auth.BAD_TIMESTAMP - digest = hmac.new( - str.encode(key), str.encode(header), hashlib.sha1 - ).hexdigest() + try: + digest = hmac.new( + str.encode(key), str.encode(header), hashlib.sha1 + ).hexdigest() + except (AttributeError, TypeError) as exc: + logger.warning( + "Failed to encode key or header for signature validation: %s", exc + ) + return Auth.BAD_SIGNATURE if signature == digest: return Auth.OK return Auth.BAD_SIGNATURE diff --git a/solarwinds_apm/traceoptions.py b/solarwinds_apm/traceoptions.py index c1e0d0919..e8e96b4e4 100644 --- a/solarwinds_apm/traceoptions.py +++ b/solarwinds_apm/traceoptions.py @@ -57,7 +57,13 @@ def __init__( # If x-trace-options header given, set response header self.include_response = True - traceoptions = re.split(r";+", xtraceoptions_header) + try: + traceoptions = re.split(r";+", xtraceoptions_header) + except (TypeError, AttributeError) as exc: + logger.debug("Failed to parse x-trace-options header: %s", exc) + self.options_header = "" + self.include_response = False + return for option in traceoptions: # KVs (e.g. sw-keys or custom-key1) are assigned by equals option_kv = option.split("=", 1) @@ -106,7 +112,7 @@ def __init__( try: if not self.timestamp: self.timestamp = int(option_kv[1]) - except ValueError: + except (ValueError, IndexError): logger.debug("ts must be base 10 int. Ignoring.") self.ignored.append(self._XTRACEOPTIONS_HEADER_KEY_TS) diff --git a/solarwinds_apm/w3c_transformer.py b/solarwinds_apm/w3c_transformer.py index 0b21f820e..b6dc0736a 100644 --- a/solarwinds_apm/w3c_transformer.py +++ b/solarwinds_apm/w3c_transformer.py @@ -7,7 +7,7 @@ """Provides functionality to transform OpenTelemetry Data to SolarWinds Observability data.""" from opentelemetry.sdk.trace import SpanContext -from opentelemetry.trace.span import TraceState +from opentelemetry.trace.span import INVALID_SPAN_ID, TraceState from solarwinds_apm.apm_constants import INTL_SWO_X_OPTIONS_RESPONSE_KEY @@ -43,9 +43,13 @@ def span_id_from_sw(cls, sw_val: str) -> str: sw_val (str): The sw tracestate value (format: "-"). Returns: - str: The span ID portion of the sw value. + str: The span ID portion of the sw value, or all-zero fallback when + input is not parseable. """ - return sw_val.split("-")[0] + try: + return sw_val.split("-")[0] + except (AttributeError, TypeError): + return cls._SPAN_ID_HEX.format(INVALID_SPAN_ID) @classmethod def trace_flags_from_int(cls, trace_flags: int) -> str: diff --git a/tests/unit/test_oboe/test_http_sampler.py b/tests/unit/test_oboe/test_http_sampler.py index 881554537..fe78852e0 100644 --- a/tests/unit/test_oboe/test_http_sampler.py +++ b/tests/unit/test_oboe/test_http_sampler.py @@ -3,6 +3,7 @@ # 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 json import os import socket from unittest.mock import patch, MagicMock @@ -189,6 +190,34 @@ def test_fetch_from_collector_success(mock_get, config, meter_provider): assert mock_get.call_count == 2 +@pytest.mark.parametrize( + "json_error", + [ + ValueError("invalid json"), + json.JSONDecodeError("invalid json", "", 0), + ], +) +@patch("requests.get") +def test_fetch_from_collector_invalid_json_returns_empty_dict_and_thread_survives( + mock_get, config, meter_provider, json_error +): + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.side_effect = json_error + mock_get.return_value = mock_response + + sampler = HttpSampler(meter_provider=meter_provider, config=config, initial=None) + try: + result = sampler._fetch_from_collector() + assert result == {} + + # Verify parse failures in periodic work do not terminate the daemon thread. + sampler._task() + assert sampler._daemon_thread.is_alive() + finally: + sampler.shutdown() + + def test_shutdown(config, meter_provider): sampler = HttpSampler(meter_provider=meter_provider, config=config, initial=None) sampler.shutdown() diff --git a/tests/unit/test_w3c_transformer.py b/tests/unit/test_w3c_transformer.py index 99f43e19d..6327604cc 100644 --- a/tests/unit/test_w3c_transformer.py +++ b/tests/unit/test_w3c_transformer.py @@ -31,6 +31,9 @@ def test_span_from_int(self): def test_span_id_from_sw(self): assert W3CTransformer.span_id_from_sw("foo-bar") == "foo" + def test_span_id_from_sw_invalid_type_returns_zero_fallback(self): + assert W3CTransformer.span_id_from_sw(None) == "{:016x}".format(0) + def test_trace_flags_from_int(self): assert W3CTransformer.trace_flags_from_int(1) == "01" diff --git a/tests/unit/test_xtraceoptions.py b/tests/unit/test_xtraceoptions.py index 6fa238669..8af1929b6 100644 --- a/tests/unit/test_xtraceoptions.py +++ b/tests/unit/test_xtraceoptions.py @@ -44,6 +44,17 @@ def test_init_signature_only(self): assert xto.timestamp == 0 assert not xto.include_response + def test_init_invalid_non_string_header_treated_as_absent(self): + xto = XTraceOptions(123, "bar") + assert xto.ignored == [] + assert xto.options_header == "" + assert xto.signature == "bar" + assert xto.custom_kvs == {} + assert xto.sw_keys == "" + assert xto.trigger_trace == 0 + assert xto.timestamp == 0 + assert not xto.include_response + def test_init_xtraceoption_and_signature(self): xto = XTraceOptions("foo", "bar") assert xto.ignored == ["foo"]