Skip to content

Commit

Permalink
Merge pull request #1288 from newrelic/update-agent-control-env-vars
Browse files Browse the repository at this point in the history
Refactor agent control config.
  • Loading branch information
umaannamalai authored Jan 27, 2025
2 parents c56dff3 + 6e9ad20 commit 300ab44
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 29 deletions.
40 changes: 21 additions & 19 deletions newrelic/core/agent_control_health.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from enum import IntEnum
from pathlib import Path
from urllib.parse import urlparse
from newrelic.core.config import _environ_as_bool, _environ_as_int


_logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -64,23 +65,18 @@ class HealthStatus(IntEnum):
def is_valid_file_delivery_location(file_uri):
# Verify whether file directory provided to agent via env var is a valid file URI to determine whether health
# check should run
if not file_uri:
_logger.warning("Configured Agent Control health delivery location is empty. Health check will not be enabled.")
return False

try:
parsed_uri = urlparse(file_uri)

if not parsed_uri.scheme or not parsed_uri.path:
_logger.warning(
"Configured Agent Control health delivery location is not a complete file URI. Health check will not be"
"Configured Agent Control health delivery location is not a complete file URI. Health check will not be "
"enabled. "
)
return False

if parsed_uri.scheme != "file":
_logger.warning(
"Configured Agent Control health delivery location does not have a valid scheme. Health check will not be"
"Configured Agent Control health delivery location does not have a valid scheme. Health check will not be "
"enabled."
)
return False
Expand Down Expand Up @@ -130,13 +126,21 @@ def __init__(self):

@property
def health_check_enabled(self):
fleet_id_present = os.environ.get("NEW_RELIC_AGENT_CONTROL_FLEET_ID", None)
if not fleet_id_present:
# Default to False - this must be explicitly set to True by the sidecar/ operator to enable health check
agent_control_enabled = _environ_as_bool("NEW_RELIC_AGENT_CONTROL_ENABLED", False)
if not agent_control_enabled:
return False

health_file_location = os.environ.get("NEW_RELIC_AGENT_CONTROL_HEALTH_DELIVERY_LOCATION", None)
return is_valid_file_delivery_location(self.health_delivery_location)

return is_valid_file_delivery_location(health_file_location)
@property
def health_delivery_location(self):
# Set a default file path if env var is not set or set to an empty string
health_file_location = (
os.environ.get("NEW_RELIC_AGENT_CONTROL_HEALTH_DELIVERY_LOCATION", "") or "file:///newrelic/apm/health"
)

return health_file_location

@property
def is_healthy(self):
Expand Down Expand Up @@ -172,15 +176,9 @@ def update_to_healthy_status(self, protocol_error=False, collector_error=False):

def write_to_health_file(self):
status_time_unix_nano = time.time_ns()
health_file_location = os.environ.get("NEW_RELIC_AGENT_CONTROL_HEALTH_DELIVERY_LOCATION", None)

# Additional safeguard though health delivery location contents were initially checked to determine if health
# check should be enabled
if not health_file_location:
return

try:
file_path = urlparse(health_file_location).path
file_path = urlparse(self.health_delivery_location).path
file_id = self.get_file_id()
file_name = f"health-{file_id}.yml"
full_path = os.path.join(file_path, file_name)
Expand Down Expand Up @@ -216,7 +214,11 @@ def agent_control_health_instance():


def agent_control_healthcheck_loop():
reporting_frequency = os.environ.get("NEW_RELIC_AGENT_CONTROL_HEALTH_FREQUENCY", 5)
reporting_frequency = _environ_as_int("NEW_RELIC_AGENT_CONTROL_HEALTH_FREQUENCY", 5)
# If we have an invalid integer value for frequency, default back to 5
if reporting_frequency <= 0:
reporting_frequency = 5

scheduler = sched.scheduler(time.time, time.sleep)

# Target this function when starting agent control health check threads to keep the scheduler running
Expand Down
25 changes: 16 additions & 9 deletions tests/agent_features/test_agent_control_health_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,22 @@ def get_health_file_contents(tmp_path):
contents = f.readlines()
return contents


@pytest.mark.parametrize("file_uri", ["", "file://", "/test/dir", "foo:/test/dir"])
def test_invalid_file_directory_supplied(file_uri):
assert is_valid_file_delivery_location(file_uri) is False
assert not is_valid_file_delivery_location(file_uri)


def test_agent_control_not_enabled(monkeypatch, tmp_path):
# Only monkeypatch a valid file URI for delivery location to test default "NEW_RELIC_AGENT_CONTROL_ENABLED" behavior
file_path = tmp_path.as_uri()
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_HEALTH_DELIVERY_LOCATION", file_path)

assert not agent_control_health_instance().health_check_enabled


def test_write_to_file_healthy_status(monkeypatch, tmp_path):
# Setup expected env vars to run agent control health check
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_FLEET_ID", "1234")
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_ENABLED", True)
file_path = tmp_path.as_uri()
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_HEALTH_DELIVERY_LOCATION", file_path)

Expand All @@ -62,7 +69,7 @@ def test_write_to_file_healthy_status(monkeypatch, tmp_path):

def test_write_to_file_unhealthy_status(monkeypatch, tmp_path):
# Setup expected env vars to run agent control health check
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_FLEET_ID", "1234")
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_ENABLED", True)
file_path = tmp_path.as_uri()
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_HEALTH_DELIVERY_LOCATION", file_path)

Expand All @@ -86,7 +93,7 @@ def test_write_to_file_unhealthy_status(monkeypatch, tmp_path):

def test_no_override_on_unhealthy_shutdown(monkeypatch, tmp_path):
# Setup expected env vars to run agent control health check
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_FLEET_ID", "1234")
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_ENABLED", True)
file_path = tmp_path.as_uri()
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_HEALTH_DELIVERY_LOCATION", file_path)

Expand All @@ -113,7 +120,7 @@ def test_health_check_running_threads(monkeypatch, tmp_path):
# Only the main thread should be running since not agent control env vars are set
assert len(running_threads) == 1

monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_FLEET_ID", "1234")
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_ENABLED", True)
file_path = tmp_path.as_uri()
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_HEALTH_DELIVERY_LOCATION", file_path)

Expand All @@ -130,7 +137,7 @@ def test_health_check_running_threads(monkeypatch, tmp_path):

def test_proxy_error_status(monkeypatch, tmp_path):
# Setup expected env vars to run agent control health check
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_FLEET_ID", "1234")
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_ENABLED", True)
file_path = tmp_path.as_uri()
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_HEALTH_DELIVERY_LOCATION", file_path)

Expand Down Expand Up @@ -163,7 +170,7 @@ def test_proxy_error_status(monkeypatch, tmp_path):

def test_update_to_healthy(monkeypatch, tmp_path):
# Setup expected env vars to run agent control health check
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_FLEET_ID", "1234")
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_ENABLED", True)
file_path = tmp_path.as_uri()
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_HEALTH_DELIVERY_LOCATION", file_path)

Expand Down Expand Up @@ -195,7 +202,7 @@ def test_update_to_healthy(monkeypatch, tmp_path):

def test_multiple_activations_running_threads(monkeypatch, tmp_path):
# Setup expected env vars to run agent control health check
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_FLEET_ID", "1234")
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_ENABLED", True)
file_path = tmp_path.as_uri()
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_HEALTH_DELIVERY_LOCATION", file_path)

Expand Down
2 changes: 1 addition & 1 deletion tests/agent_unittests/test_agent_connect.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def test_ml_streaming_disabled_supportability_metrics():
)
def test_agent_control_health_supportability_metric(monkeypatch, tmp_path):
# Setup expected env vars to run agent control health check
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_FLEET_ID", "1234")
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_ENABLED", True)
file_path = tmp_path.as_uri()
monkeypatch.setenv("NEW_RELIC_AGENT_CONTROL_HEALTH_DELIVERY_LOCATION", file_path)

Expand Down

0 comments on commit 300ab44

Please sign in to comment.