Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
from opentelemetry.trace import set_tracer_provider

from appsignal import probes
from appsignal.agent import agent
from appsignal.check_in.heartbeat import (
_kill_continuous_heartbeats,
_reset_heartbeat_continuous_interval_seconds,
Expand Down Expand Up @@ -105,11 +104,6 @@ def stop_and_clear_probes_after_tests() -> Any:
probes.clear()


@pytest.fixture(scope="function", autouse=True)
def reset_agent_active_state() -> Any:
agent.active = False
Comment on lines -108 to -110

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to reset this anymore between tests?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think not! There's not an agent global anymore -- instead, each client has its own agent instance. We do reset the global client between tests, so the new one that is initialised should be in a clean state.



@pytest.fixture(scope="function", autouse=True)
def reset_global_client() -> Any:
_reset_client()
Expand Down
30 changes: 24 additions & 6 deletions src/appsignal/agent.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,27 @@
from __future__ import annotations

import os
import signal
import subprocess
import time
from dataclasses import dataclass
from pathlib import Path

from . import internal_logger as logger
from .binary import Binary
from .config import Config


@dataclass
class Agent:
class Agent(Binary):
package_path: Path = Path(__file__).parent
agent_path: Path = package_path / "appsignal-agent"
platform_path: Path = package_path / "_appsignal_platform"
active: bool = False
_active: bool = False

@property
def active(self) -> bool:
return self._active

def start(self, config: Config) -> None:
config.set_private_environ()
Expand All @@ -33,12 +42,24 @@ def start(self, config: Config) -> None:
p.wait(timeout=1)
returncode = p.returncode
if returncode == 0:
self.active = True
self._active = True
else:
output, _ = p.communicate()
out = output.decode("utf-8")
print(f"AppSignal agent is unable to start ({returncode}): ", out)

def stop(self, config: Config) -> None:
working_dir = config.option("working_directory_path") or "/tmp/appsignal"
lock_path = os.path.join(working_dir, "agent.lock")
try:
with open(lock_path) as file:
line = file.readline()
pid = int(line.split(";")[2])
os.kill(pid, signal.SIGTERM)
time.sleep(2)
except FileNotFoundError:
logger.info("Agent lock file not found; not stopping the agent")

def diagnose(self, config: Config) -> bytes:
config.set_private_environ()
return subprocess.run(
Expand All @@ -56,6 +77,3 @@ def architecture_and_platform(self) -> list[str]:
return file.read().split("-", 1)
except FileNotFoundError:
return ["", ""]


agent = Agent()
32 changes: 32 additions & 0 deletions src/appsignal/binary.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
from __future__ import annotations

from abc import ABC, abstractmethod

from .config import Config


class Binary(ABC):
@property
@abstractmethod
def active(self) -> bool: ...

@abstractmethod
def start(self, config: Config) -> None: ...

@abstractmethod
def stop(self, config: Config) -> None: ...


class NoopBinary(Binary):
def __init__(self, active: bool = False) -> None:
self._active = active

@property
def active(self) -> bool:
return self._active

def start(self, config: Config) -> None:
pass

def stop(self, config: Config) -> None:
pass
41 changes: 25 additions & 16 deletions src/appsignal/client.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
from __future__ import annotations

import os
import signal
import time
from typing import TYPE_CHECKING

from . import internal_logger as logger
from .agent import agent
from .binary import NoopBinary
from .config import Config, Options
from .opentelemetry import start as start_opentelemetry
from .probes import start as start_probes
Expand All @@ -15,6 +12,8 @@
if TYPE_CHECKING:
from typing_extensions import Unpack

from .binary import Binary


_client: Client | None = None

Expand All @@ -26,11 +25,13 @@ def _reset_client() -> None:

class Client:
_config: Config
_binary: Binary

def __init__(self, **options: Unpack[Options]) -> None:
global _client

self._config = Config(options)
self._binary = NoopBinary()
_client = self

@classmethod
Expand All @@ -41,10 +42,12 @@ def config(cls) -> Config | None:
return _client._config

def start(self) -> None:
self._set_binary()

if self._config.is_active():
logger.info("Starting AppSignal")
agent.start(self._config)
if not agent.active:
self._binary.start(self._config)
if not self._binary.active:
return
start_opentelemetry(self._config)
self._start_probes()
Expand All @@ -56,17 +59,23 @@ def stop(self) -> None:

logger.info("Stopping AppSignal")
scheduler().stop()
working_dir = self._config.option("working_directory_path") or "/tmp/appsignal"
lock_path = os.path.join(working_dir, "agent.lock")
try:
with open(lock_path) as file:
line = file.readline()
pid = int(line.split(";")[2])
os.kill(pid, signal.SIGTERM)
time.sleep(2)
except FileNotFoundError:
logger.info("Agent lock file not found")
self._binary.stop(self._config)

def _start_probes(self) -> None:
if self._config.option("enable_minutely_probes"):
start_probes()

def _set_binary(self) -> None:
if self._config.should_use_external_collector():
# When a custom collector endpoint is set, use a `NoopBinary`
# set to active, so that OpenTelemetry and probes are started,
# but the agent is not started.
logger.info(
"Not starting the AppSignal agent: using collector endpoint instead"
)
self._binary = NoopBinary(active=True)
else:
# Use the agent when a custom collector endpoint is not set.
from .agent import Agent

self._binary = Agent()
48 changes: 32 additions & 16 deletions src/appsignal/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class Options(TypedDict, total=False):
app_path: str | None
bind_address: str | None
ca_file_path: str | None
collector_endpoint: str | None
cpu_count: float | None
diagnose_endpoint: str | None
disable_default_instrumentations: None | (
Expand Down Expand Up @@ -48,6 +49,7 @@ class Options(TypedDict, total=False):
send_environment_metadata: bool | None
send_params: bool | None
send_session_data: bool | None
service_name: str | None
statsd_port: str | int | None
working_directory_path: str | None

Expand Down Expand Up @@ -97,22 +99,23 @@ class Config:
)

DefaultInstrumentation = Literal[
"opentelemetry.instrumentation.aiopg",
"opentelemetry.instrumentation.asyncpg",
"opentelemetry.instrumentation.celery",
"opentelemetry.instrumentation.django",
"opentelemetry.instrumentation.flask",
"opentelemetry.instrumentation.jinja2",
"opentelemetry.instrumentation.mysql",
"opentelemetry.instrumentation.mysqlclient",
"opentelemetry.instrumentation.pika",
"opentelemetry.instrumentation.psycopg",
"opentelemetry.instrumentation.psycopg2",
"opentelemetry.instrumentation.pymysql",
"opentelemetry.instrumentation.redis",
"opentelemetry.instrumentation.requests",
"opentelemetry.instrumentation.sqlite3",
"opentelemetry.instrumentation.sqlalchemy",
"aiopg",
"asyncpg",
"celery",
"django",
"flask",
"jinja2",
"mysql",
"mysqlclient",
"pika",
"psycopg",
"psycopg2",
"pymysql",
"redis",
"requests",
"sqlite3",
"sqlalchemy",
"logging",
]
DEFAULT_INSTRUMENTATIONS = cast(
List[DefaultInstrumentation], list(get_args(DefaultInstrumentation))
Expand Down Expand Up @@ -140,6 +143,17 @@ def is_active(self) -> bool:
def option(self, option: str) -> Any:
return self.options.get(option)

# Whether it should instrument logging.
def should_instrument_logging(self) -> bool:
# The agent does not support logging, so this is equivalent
# to whether it should use an external collector.
return self.should_use_external_collector()

# Whether it should use a collector to send data to AppSignal
# which is booted externally to this integration.
def should_use_external_collector(self) -> bool:
return self.option("collector_endpoint") is not None

@staticmethod
def load_from_system() -> Options:
return Options(app_path=os.getcwd())
Expand All @@ -150,6 +164,7 @@ def load_from_environment() -> Options:
active=parse_bool(os.environ.get("APPSIGNAL_ACTIVE")),
bind_address=os.environ.get("APPSIGNAL_BIND_ADDRESS"),
ca_file_path=os.environ.get("APPSIGNAL_CA_FILE_PATH"),
collector_endpoint=os.environ.get("APPSIGNAL_COLLECTOR_ENDPOINT"),
cpu_count=parse_float(os.environ.get("APPSIGNAL_CPU_COUNT")),
diagnose_endpoint=os.environ.get("APPSIGNAL_DIAGNOSE_ENDPOINT"),
disable_default_instrumentations=parse_disable_default_instrumentations(
Expand Down Expand Up @@ -199,6 +214,7 @@ def load_from_environment() -> Options:
),
send_params=parse_bool(os.environ.get("APPSIGNAL_SEND_PARAMS")),
send_session_data=parse_bool(os.environ.get("APPSIGNAL_SEND_SESSION_DATA")),
service_name=os.environ.get("APPSIGNAL_SERVICE_NAME"),
statsd_port=os.environ.get("APPSIGNAL_STATSD_PORT"),
working_directory_path=os.environ.get("APPSIGNAL_WORKING_DIRECTORY_PATH"),
)
Expand Down
3 changes: 3 additions & 0 deletions src/appsignal/internal_logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ def _configure_logger() -> tuple[logging.Logger, str]:

logger = logging.getLogger("appsignal")
logger.setLevel(_LOG_LEVEL_MAPPING[level])
# Prevent internal logger messages from propagating to the root logger
# (which will have an OpenTelemetry handler attached)
logger.propagate = False

if config.option("log") == "file":
log_file_path = config.log_file_path()
Expand Down
Loading
Loading