No need for functions and methods to be private since _opamp module is already private

Author: xrmx

opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/__init__.py

@@ -46,7 +46,6 @@
 
     import os
 
-    from opentelemetry._opamp import messages
     from opentelemetry._opamp.agent import OpAMPAgent
     from opentelemetry._opamp.client import OpAMPClient
     from opentelemetry._opamp.proto import opamp_pb2 as opamp_pb2
@@ -55,7 +54,7 @@
 
 
     def opamp_handler(agent: OpAMPAgent, client: OpAMPClient, message: opamp_pb2.ServerToAgent):
-        for config_filename, config in messages._decode_remote_config(message.remote_config):
+        for config_filename, config in client.decode_remote_config(message.remote_config):
             print("do something")
 
 

opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/agent.py

@@ -124,7 +124,7 @@ def start(self) -> None:
         atexit.register(self.stop)
 
         # enqueue the connection message so we can then enable heartbeat
-        payload = self._client._build_connection_message()
+        payload = self._client.build_connection_message()
         self.send(
             payload,
             max_retries=self._max_retries,
@@ -162,7 +162,7 @@ def _run_scheduler(self) -> None:
         """
         while not self._stop.wait(self._interval):
             if self._schedule:
-                payload = self._client._build_heartbeat_message()
+                payload = self._client.build_heartbeat_message()
                 job = _Job(
                     payload=payload,
                     max_retries=self._heartbeat_max_retries,
@@ -185,7 +185,7 @@ def _run_worker(self) -> None:
             message = None
             while job.should_retry() and not self._stop.is_set():
                 try:
-                    message = self._client._send(job.payload)
+                    message = self._client.send(job.payload)
                     logger.debug("Job succeeded: %r", job.payload)
                     break
                 except Exception as exc:
@@ -241,9 +241,9 @@ def stop(self) -> None:
         # Before exiting send signal the server we are disconnecting to free our resources
         # This is not required by the spec but is helpful in practice
         logger.debug("Stopping OpAMPClient: sending AgentDisconnect")
-        payload = self._client._build_agent_disconnect_message()
+        payload = self._client.build_agent_disconnect_message()
         try:
-            self._client._send(payload)
+            self._client.send(payload)
         except Exception:  # pylint: disable=broad-exception-caught
             logger.debug(
                 "Stopping OpAMPClient: failed to send AgentDisconnect message"

opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/client.py

@@ -73,43 +73,43 @@ def __init__(
         headers = headers or {}
         self._headers = {**_OTLP_HTTP_HEADERS, **headers}
 
-        self._agent_description = messages._build_agent_description(
+        self._agent_description = messages.build_agent_description(
             identifying_attributes=agent_identifying_attributes,
             non_identifying_attributes=agent_non_identifying_attributes,
         )
         self._sequence_num: int = 0
         self._instance_uid: bytes = uuid7().bytes
         self._remote_config_status: opamp_pb2.RemoteConfigStatus | None = None
 
-    def _build_connection_message(self) -> bytes:
-        message = messages._build_presentation_message(
+    def build_connection_message(self) -> bytes:
+        message = messages.build_presentation_message(
             instance_uid=self._instance_uid,
             agent_description=self._agent_description,
             sequence_num=self._sequence_num,
             capabilities=_HANDLED_CAPABILITIES,
         )
-        data = messages._encode_message(message)
+        data = messages.encode_message(message)
         return data
 
-    def _build_agent_disconnect_message(self) -> bytes:
-        message = messages._build_agent_disconnect_message(
+    def build_agent_disconnect_message(self) -> bytes:
+        message = messages.build_agent_disconnect_message(
             instance_uid=self._instance_uid,
             sequence_num=self._sequence_num,
             capabilities=_HANDLED_CAPABILITIES,
         )
-        data = messages._encode_message(message)
+        data = messages.encode_message(message)
         return data
 
-    def _build_heartbeat_message(self) -> bytes:
-        message = messages._build_heartbeat_message(
+    def build_heartbeat_message(self) -> bytes:
+        message = messages.build_heartbeat_message(
             instance_uid=self._instance_uid,
             sequence_num=self._sequence_num,
             capabilities=_HANDLED_CAPABILITIES,
         )
-        data = messages._encode_message(message)
+        data = messages.encode_message(message)
         return data
 
-    def _update_remote_config_status(
+    def update_remote_config_status(
         self,
         remote_config_hash: bytes,
         status: opamp_pb2.RemoteConfigStatuses.ValueType,
@@ -129,7 +129,7 @@ def _update_remote_config_status(
                 remote_config_hash,
             )
             self._remote_config_status = (
-                messages._build_remote_config_status_message(
+                messages.build_remote_config_status_message(
                     last_remote_config_hash=remote_config_hash,
                     status=status,
                     error_message=error_message,
@@ -139,19 +139,19 @@ def _update_remote_config_status(
 
         return None
 
-    def _build_remote_config_status_response_message(
+    def build_remote_config_status_response_message(
         self, remote_config_status: opamp_pb2.RemoteConfigStatus
     ) -> bytes:
-        message = messages._build_remote_config_status_response_message(
+        message = messages.build_remote_config_status_response_message(
             instance_uid=self._instance_uid,
             sequence_num=self._sequence_num,
             capabilities=_HANDLED_CAPABILITIES,
             remote_config_status=remote_config_status,
         )
-        data = messages._encode_message(message)
+        data = messages.encode_message(message)
         return data
 
-    def _send(self, data: bytes):
+    def send(self, data: bytes):
         token = attach(set_value(_SUPPRESS_INSTRUMENTATION_KEY, True))
         try:
             response = self._transport.send(
@@ -166,10 +166,10 @@ def _send(self, data: bytes):
             detach(token)
 
     @staticmethod
-    def _decode_remote_config(
+    def decode_remote_config(
         remote_config: opamp_pb2.AgentRemoteConfig,
     ) -> Generator[tuple[str, Mapping[str, AnyValue]]]:
-        for config_file, config in messages._decode_remote_config(
+        for config_file, config in messages.decode_remote_config(
             remote_config
         ):
             yield config_file, config

opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/messages.py

@@ -13,8 +13,6 @@
 # limitations under the License.
 
 # pylint: disable=no-name-in-module
-# FIXME: remove this after _opamp -> opamp, making this helpers public is not enough for pyright
-# type: ignore[reportUnusedFunction]
 
 from __future__ import annotations
 
@@ -35,7 +33,7 @@
 from opentelemetry.util.types import AnyValue
 
 
-def _decode_message(data: bytes) -> opamp_pb2.ServerToAgent:
+def decode_message(data: bytes) -> opamp_pb2.ServerToAgent:
     message = opamp_pb2.ServerToAgent()
     message.ParseFromString(data)
     return message
@@ -65,7 +63,7 @@ def _encode_attributes(attributes: Mapping[str, AnyValue]):
     ]
 
 
-def _build_agent_description(
+def build_agent_description(
     identifying_attributes: Mapping[str, AnyValue],
     non_identifying_attributes: Mapping[str, AnyValue] | None = None,
 ) -> opamp_pb2.AgentDescription:
@@ -81,7 +79,7 @@ def _build_agent_description(
     )
 
 
-def _build_presentation_message(
+def build_presentation_message(
     instance_uid: bytes,
     sequence_num: int,
     agent_description: opamp_pb2.AgentDescription,
@@ -96,7 +94,7 @@ def _build_presentation_message(
     return command
 
 
-def _build_heartbeat_message(
+def build_heartbeat_message(
     instance_uid: bytes, sequence_num: int, capabilities: int
 ) -> opamp_pb2.AgentToServer:
     command = opamp_pb2.AgentToServer(
@@ -107,7 +105,7 @@ def _build_heartbeat_message(
     return command
 
 
-def _build_agent_disconnect_message(
+def build_agent_disconnect_message(
     instance_uid: bytes, sequence_num: int, capabilities: int
 ) -> opamp_pb2.AgentToServer:
     command = opamp_pb2.AgentToServer(
@@ -119,7 +117,7 @@ def _build_agent_disconnect_message(
     return command
 
 
-def _build_remote_config_status_message(
+def build_remote_config_status_message(
     last_remote_config_hash: bytes,
     status: opamp_pb2.RemoteConfigStatuses.ValueType,
     error_message: str = "",
@@ -131,7 +129,7 @@ def _build_remote_config_status_message(
     )
 
 
-def _build_remote_config_status_response_message(
+def build_remote_config_status_response_message(
     instance_uid: bytes,
     sequence_num: int,
     capabilities: int,
@@ -146,11 +144,11 @@ def _build_remote_config_status_response_message(
     return command
 
 
-def _encode_message(data: opamp_pb2.AgentToServer) -> bytes:
+def encode_message(data: opamp_pb2.AgentToServer) -> bytes:
     return data.SerializeToString()
 
 
-def _decode_remote_config(
+def decode_remote_config(
     remote_config: opamp_pb2.AgentRemoteConfig,
 ) -> Generator[tuple[str, Mapping[str, AnyValue]]]:
     for (

opamp/opentelemetry-opamp-client/src/opentelemetry/_opamp/transport/requests.py

@@ -44,6 +44,6 @@ def send(
         except Exception:
             raise OpAMPException
 
-        message = messages._decode_message(response.content)
+        message = messages.decode_message(response.content)
 
         return message

opamp/opentelemetry-opamp-client/tests/opamp/test_agent.py

@@ -38,7 +38,7 @@ def test_can_start_agent():
 def test_agent_start_will_send_connection_and_disconnetion_messages():
     client_mock = mock.Mock()
     mock_message = {"mock": "message"}
-    client_mock._send.return_value = mock_message
+    client_mock.send.return_value = mock_message
     message_handler = mock.Mock()
     agent = OpAMPAgent(
         interval=30, client=client_mock, message_handler=message_handler
@@ -49,7 +49,7 @@ def test_agent_start_will_send_connection_and_disconnetion_messages():
     agent.stop()
 
     # one send for connection message, one for disconnect agent message
-    assert client_mock._send.call_count == 2
+    assert client_mock.send.call_count == 2
     # connection callback has been called
     assert agent._schedule is True
     # connection message response has been received
@@ -92,7 +92,7 @@ def test_agent_retries_before_max_attempts(caplog):
     message_handler_mock = mock.Mock()
     client_mock = mock.Mock()
     connection_message = disconnection_message = server_message = mock.Mock()
-    client_mock._send.side_effect = [
+    client_mock.send.side_effect = [
         connection_message,
         Exception,
         server_message,
@@ -110,7 +110,7 @@ def test_agent_retries_before_max_attempts(caplog):
     sleep(0.1)
     agent.stop()
 
-    assert client_mock._send.call_count == 4
+    assert client_mock.send.call_count == 4
     assert message_handler_mock.call_count == 2
 
 
@@ -119,7 +119,7 @@ def test_agent_stops_after_max_attempts(caplog):
     message_handler_mock = mock.Mock()
     client_mock = mock.Mock()
     connection_message = disconnection_message = mock.Mock()
-    client_mock._send.side_effect = [
+    client_mock.send.side_effect = [
         connection_message,
         Exception,
         Exception,
@@ -138,7 +138,7 @@ def test_agent_stops_after_max_attempts(caplog):
     sleep(0.1)
     agent.stop()
 
-    assert client_mock._send.call_count == 4
+    assert client_mock.send.call_count == 4
     assert message_handler_mock.call_count == 1