Skip to content

Commit

Permalink
separate instance config from collector output config
Browse files Browse the repository at this point in the history
  • Loading branch information
ankona committed Feb 22, 2024
1 parent 45361be commit 2133628
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 15 deletions.
1 change: 1 addition & 0 deletions smartsim/_core/control/job.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ def __init__(self) -> None:
self.status_dir: str = ""
self.telemetry_on: bool = False
self.collectors: t.Dict[str, str] = {}
self.config: t.Dict[str, str] = {}

@property
def is_db(self) -> bool:
Expand Down
35 changes: 26 additions & 9 deletions smartsim/_core/entrypoints/telemetrymonitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,23 @@ class _Address:
host: str
port: int

def __init__(self, host: str, port: int) -> None:
"""Initialize the instance"""
self.host = host.strip() if host else ""
self.port = port
_Address._check(self.host, self.port)


@staticmethod
def _check(host: str, port: int) -> None:
"""Validate input arguments"""
if not host:
raise ValueError("Address requires host")
if not port:
raise ValueError("Address requires port")

def __str__(self) -> str:
"""Pretty-print the address"""
"""Pretty-print the instance"""
return f"{self.host}:{self.port}"


Expand All @@ -185,8 +200,8 @@ def __init__(self, entity: JobEntity, sink: Sink) -> None:
super().__init__(entity, sink)
self._client: t.Optional[redis.Redis[bytes]] = None
self._address = _Address(
self._entity.collectors.get("host", "127.0.0.1"),
int(self._entity.collectors.get("port", 6379)),
self._entity.config.get("host", ""),
int(self._entity.config.get("port", 0)),
)

async def _configure_client(self) -> None:
Expand Down Expand Up @@ -485,13 +500,15 @@ def _hydrate_persistable(
if entity.is_db:
# db shards are hydrated individually
entity.telemetry_on = int(persistable_entity.get("collectors", "0")) > 0
cfg = entity.collectors
col_cfg = entity.collectors

if entity.telemetry_on:
cfg["host"] = persistable_entity.get("hostname", "NOT-SET")
cfg["port"] = persistable_entity.get("port", "NOT-SET")
cfg["client"] = persistable_entity.get("client_file", "")
cfg["client_count"] = persistable_entity.get("client_count_file", "")
cfg["memory"] = persistable_entity.get("memory_file", "")
col_cfg["client"] = persistable_entity.get("client_file", "")
col_cfg["client_count"] = persistable_entity.get("client_count_file", "")
col_cfg["memory"] = persistable_entity.get("memory_file", "")

entity.config["host"] = persistable_entity.get("hostname", "")
entity.config["port"] = persistable_entity.get("port", "")

return entity

Expand Down
6 changes: 4 additions & 2 deletions tests/test_collector_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,14 @@ def _mock_entity(
entity.type = type
entity.telemetry_on = True
entity.collectors = {
"host": host,
"port": port,
"client": "",
"client_count": "",
"memory": "",
}
entity.config = {
"host": host,
"port": port,
}
return entity

return _mock_entity
Expand Down
6 changes: 4 additions & 2 deletions tests/test_collector_sink.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,14 @@ def _mock_entity(
entity.type = type
entity.telemetry_on = True
entity.collectors = {
"host": host,
"port": port,
"client": "",
"client_count": "",
"memory": "",
}
entity.config = {
"host": host,
"port": port,
}
return entity

return _mock_entity
Expand Down
29 changes: 27 additions & 2 deletions tests/test_collectors.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,14 @@ def _mock_entity(
entity.type = type
entity.telemetry_on = True
entity.collectors = {
"host": host,
"port": port,
"client": "",
"client_count": "",
"memory": "",
}
entity.config = {
"host": host,
"port": port,
}
return entity

return _mock_entity
Expand Down Expand Up @@ -94,6 +96,29 @@ async def test_dbmemcollector_prepare_fail(
assert collector.value is None


@pytest.mark.asyncio
async def test_dbcollector_config(
mock_entity,
mock_sink,
monkeypatch: pytest.MonkeyPatch,
):
"""Ensure that missing required db collector config causes an exception"""

# Check that a bad host causes exception
entity = mock_entity(host="")
with pytest.raises(ValueError):
DbMemoryCollector(entity, mock_sink())

entity = mock_entity(host=" ")
with pytest.raises(ValueError):
DbMemoryCollector(entity, mock_sink())

# Check that a bad port causes exception
entity = mock_entity(port="")
with pytest.raises(ValueError):
DbMemoryCollector(entity, mock_sink())


@pytest.mark.asyncio
async def test_dbmemcollector_prepare_fail_dep(
mock_entity,
Expand Down

0 comments on commit 2133628

Please sign in to comment.