diff --git a/opamp/opentelemetry-opamp-client/.changelog/4536.added b/opamp/opentelemetry-opamp-client/.changelog/4536.added new file mode 100644 index 0000000000..8cc3d25479 --- /dev/null +++ b/opamp/opentelemetry-opamp-client/.changelog/4536.added @@ -0,0 +1 @@ +`opentelemetry-opamp-client`: support non-JSON effective config bodies diff --git a/opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/client.py b/opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/client.py index bff115374b..63ecee57c0 100644 --- a/opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/client.py +++ b/opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/client.py @@ -4,7 +4,7 @@ from __future__ import annotations from logging import getLogger -from typing import Generator, Mapping +from typing import Any, Generator, Mapping from uuid_utils import uuid7 @@ -98,7 +98,9 @@ def build_heartbeat_message(self) -> bytes: return data def update_effective_config( - self, effective_config: dict[str, dict[str, str]], content_type: str + self, + effective_config: Mapping[str, Any], + content_type: str, ) -> opamp_pb2.EffectiveConfig: self._effective_config = messages.build_effective_config_message( config=effective_config, content_type=content_type diff --git a/opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/messages.py b/opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/messages.py index 405b655d8f..37dd4b8cb2 100644 --- a/opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/messages.py +++ b/opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/messages.py @@ -6,7 +6,8 @@ from __future__ import annotations import json -from typing import Generator, Mapping +from logging import getLogger +from typing import Any, Generator, Mapping from opentelemetry._opamp.exceptions import ( OpAMPRemoteConfigDecodeException, @@ -21,6 +22,8 @@ ) from opentelemetry.util.types import AnyValue +_logger = getLogger(__name__) + def decode_message(data: bytes) -> opamp_pb2.ServerToAgent: message = opamp_pb2.ServerToAgent() @@ -119,19 +122,47 @@ def build_remote_config_status_response_message( def build_effective_config_message( - config: dict[str, dict[str, str]], content_type: str + config: Mapping[str, Any], content_type: str ): agent_config_map = opamp_pb2.AgentConfigMap() for filename, value in config.items(): - if content_type == "application/json": - body = json.dumps(value) - agent_config_map.config_map[filename].body = body.encode("utf-8") - agent_config_map.config_map[filename].content_type = content_type + body = encode_effective_config_body(content_type, value) + if body is None: + _logger.warning( + "Skipping effective config entry %s with content type %s " + "because the value cannot be encoded", + filename, + content_type, + ) + continue + + config_entry = agent_config_map.config_map[filename] + config_entry.body = body + config_entry.content_type = content_type return opamp_pb2.EffectiveConfig( config_map=agent_config_map, ) +def encode_effective_config_body( + content_type: str, value: Any +) -> bytes | None: + if content_type == "application/json": + try: + return json.dumps(value).encode("utf-8") + except (TypeError, ValueError): + _logger.warning( + "Failed to encode effective config body as JSON", + exc_info=True, + ) + return None + if isinstance(value, str): + return value.encode("utf-8") + if isinstance(value, bytes): + return value + return None + + def build_full_state_message( instance_uid: bytes, sequence_num: int, diff --git a/opamp/opentelemetry-opamp-client/tests/opamp/test_client.py b/opamp/opentelemetry-opamp-client/tests/opamp/test_client.py index 6a446eccd3..7492d83039 100644 --- a/opamp/opentelemetry-opamp-client/tests/opamp/test_client.py +++ b/opamp/opentelemetry-opamp-client/tests/opamp/test_client.py @@ -4,6 +4,7 @@ # pylint: disable=no-name-in-module import json +import logging from unittest import mock import pytest @@ -307,6 +308,68 @@ def test_update_effective_config_json_content_type(client): assert config == decoded_config +def test_update_effective_config_skips_non_serializable_json_content( + client, caplog +): + caplog.set_level(logging.WARNING, logger="opentelemetry._opamp.messages") + effective_config = client.update_effective_config( + {"config": {"a": object()}}, + content_type="application/json", + ) + assert effective_config.config_map.config_map == {} + message = "Failed to encode effective config body as JSON" + exception_records = [ + record for record in caplog.records if message in record.getMessage() + ] + assert len(exception_records) == 1 + assert exception_records[0].exc_info + assert exception_records[0].exc_info[0] is TypeError + assert "Skipping effective config entry config" in caplog.text + + +def test_update_effective_config_text_content_type(client): + config = {"config": "FEATURE_ENABLED=false\n"} + effective_config = client.update_effective_config( + config, + content_type="text/plain", + ) + config_entry = effective_config.config_map.config_map["config"] + assert config_entry.content_type == "text/plain" + assert config_entry.body == b"FEATURE_ENABLED=false\n" + + +def test_update_effective_config_bytes_content(client): + config = {"config": b"FEATURE_ENABLED=false\n"} + effective_config = client.update_effective_config( + config, + content_type="text/plain", + ) + config_entry = effective_config.config_map.config_map["config"] + assert config_entry.content_type == "text/plain" + assert config_entry.body == b"FEATURE_ENABLED=false\n" + + +def test_update_effective_config_skips_unencodable_content(client, caplog): + caplog.set_level(logging.WARNING, logger="opentelemetry._opamp.messages") + effective_config = client.update_effective_config( + {"config": {"a": "config"}}, + content_type="text/plain", + ) + assert effective_config.config_map.config_map == {} + assert "Skipping effective config entry config" in caplog.text + + +def test_build_full_state_message_text_effective_config(client): + config = {"config": "FEATURE_ENABLED=false\n"} + client.update_effective_config(config, content_type="text/plain") + data = client.build_full_state_message() + message = opamp_pb2.AgentToServer() + message.ParseFromString(data) + config_entry = message.effective_config.config_map.config_map["config"] + assert config_entry.content_type == "text/plain" + assert config_entry.body == b"FEATURE_ENABLED=false\n" + + def test_build_full_state_message(client): client.update_remote_config_status( remote_config_hash=b"12345678",