Skip to content

Commit a0e4e1c

Browse files
khvn26khvn26
committed
fix: move sys.path and DJANGO_SETTINGS_MODULE before OTel setup
DjangoInstrumentor accesses settings.MIDDLEWARE during instrument(), which requires the project directory on sys.path and DJANGO_SETTINGS_MODULE set. Previously these were set after the OTel block, causing the instrumentor to call settings.configure() with empty defaults and poisoning Django for the rest of the process. Also migrates test_main to pytest-mock (MockerFixture) and monkeypatch.setenv. Fixes #191
1 parent 396f138 commit a0e4e1c

File tree

2 files changed

+106
-82
lines changed

2 files changed

+106
-82
lines changed

‎src/common/core/main.py‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,24 @@ def ensure_cli_env() -> typing.Generator[None, None, None]:
3737
"""
3838
ctx = contextlib.ExitStack()
3939

40+
# Currently we don't install Flagsmith modules as a package, so we need to add
41+
# $CWD to the Python path to be able to import them
42+
sys.path.append(os.getcwd())
43+
44+
# TODO @khvn26 We should find a better way to pre-set the Django settings module
45+
# without resorting to it being set outside of the application
46+
os.environ.setdefault("DJANGO_SETTINGS_MODULE", "settings.dev")
47+
48+
if "docgen" in sys.argv:
49+
os.environ["DOCGEN_MODE"] = "true"
50+
51+
if "task-processor" in sys.argv:
52+
# A hacky way to signal we're not running the API
53+
os.environ["RUN_BY_PROCESSOR"] = "true"
54+
4055
# Set up OTel instrumentation (opt-in via OTEL_EXPORTER_OTLP_ENDPOINT).
56+
# Must come after sys.path / DJANGO_SETTINGS_MODULE setup because
57+
# DjangoInstrumentor accesses settings.MIDDLEWARE.
4158
otel_processors = None
4259
otel_endpoint = env.str("OTEL_EXPORTER_OTLP_ENDPOINT", None)
4360
if otel_endpoint:
@@ -84,21 +101,6 @@ def ensure_cli_env() -> typing.Generator[None, None, None]:
84101
if not os.environ.get("PROMETHEUS_MULTIPROC_DIR"):
85102
os.environ["PROMETHEUS_MULTIPROC_DIR"] = mkdtemp(prefix="flagsmith-prometheus-")
86103

87-
# Currently we don't install Flagsmith modules as a package, so we need to add
88-
# $CWD to the Python path to be able to import them
89-
sys.path.append(os.getcwd())
90-
91-
# TODO @khvn26 We should find a better way to pre-set the Django settings module
92-
# without resorting to it being set outside of the application
93-
os.environ.setdefault("DJANGO_SETTINGS_MODULE", "settings.dev")
94-
95-
if "docgen" in sys.argv:
96-
os.environ["DOCGEN_MODE"] = "true"
97-
98-
if "task-processor" in sys.argv:
99-
# A hacky way to signal we're not running the API
100-
os.environ["RUN_BY_PROCESSOR"] = "true"
101-
102104
with ctx:
103105
yield
104106

‎tests/unit/common/core/test_main.py‎

Lines changed: 89 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,30 @@
11
import os
2-
from unittest.mock import ANY, MagicMock, patch
32

3+
import pytest
44
from opentelemetry.sdk._logs import LoggerProvider
55
from opentelemetry.sdk.trace import TracerProvider
6+
from pytest_mock import MockerFixture
67
from structlog.typing import Processor
78

89
from common.core.main import ensure_cli_env
910
from common.core.otel import add_otel_trace_context
1011

1112

12-
def test_ensure_cli_env__env_vars_set__calls_setup_logging_with_env_values() -> None:
13+
def test_ensure_cli_env__env_vars_set__calls_setup_logging_with_env_values(
14+
monkeypatch: pytest.MonkeyPatch,
15+
mocker: MockerFixture,
16+
) -> None:
1317
# Given
14-
env = {
15-
"LOG_LEVEL": "WARNING",
16-
"LOG_FORMAT": "json",
17-
"LOGGING_CONFIGURATION_FILE": "/tmp/logging.json",
18-
"APPLICATION_LOGGERS": "myapp,mylib",
19-
"ACCESS_LOG_EXTRA_ITEMS": "{flagsmith.route}e,{origin}i",
20-
}
18+
monkeypatch.setenv("LOG_LEVEL", "WARNING")
19+
monkeypatch.setenv("LOG_FORMAT", "json")
20+
monkeypatch.setenv("LOGGING_CONFIGURATION_FILE", "/tmp/logging.json")
21+
monkeypatch.setenv("APPLICATION_LOGGERS", "myapp,mylib")
22+
monkeypatch.setenv("ACCESS_LOG_EXTRA_ITEMS", "{flagsmith.route}e,{origin}i")
23+
24+
mock_setup = mocker.patch("common.core.main.setup_logging")
2125

2226
# When
23-
with (
24-
patch.dict(os.environ, env),
25-
patch("common.core.main.setup_logging") as mock_setup,
26-
ensure_cli_env(),
27-
):
27+
with ensure_cli_env():
2828
pass
2929

3030
# Then
@@ -33,18 +33,18 @@ def test_ensure_cli_env__env_vars_set__calls_setup_logging_with_env_values() ->
3333
log_format="json",
3434
logging_configuration_file="/tmp/logging.json",
3535
application_loggers=["myapp", "mylib"],
36-
extra_foreign_processors=ANY,
36+
extra_foreign_processors=mocker.ANY,
3737
otel_processors=None,
3838
)
3939

4040

41-
def test_ensure_cli_env__no_env_vars__calls_setup_logging_with_defaults() -> None:
41+
def test_ensure_cli_env__no_env_vars__calls_setup_logging_with_defaults(
42+
mocker: MockerFixture,
43+
) -> None:
4244
# Given / When
43-
with (
44-
patch.dict(os.environ, {}),
45-
patch("common.core.main.setup_logging") as mock_setup,
46-
ensure_cli_env(),
47-
):
45+
mock_setup = mocker.patch("common.core.main.setup_logging")
46+
47+
with ensure_cli_env():
4848
pass
4949

5050
# Then
@@ -53,37 +53,39 @@ def test_ensure_cli_env__no_env_vars__calls_setup_logging_with_defaults() -> Non
5353
log_format="generic",
5454
logging_configuration_file=None,
5555
application_loggers=None,
56-
extra_foreign_processors=ANY,
56+
extra_foreign_processors=mocker.ANY,
5757
otel_processors=None,
5858
)
5959

6060

61-
def test_ensure_cli_env__otel_endpoint_set__configures_otel_processors() -> None:
61+
def test_ensure_cli_env__otel_endpoint_set__configures_otel_processors(
62+
monkeypatch: pytest.MonkeyPatch,
63+
mocker: MockerFixture,
64+
) -> None:
6265
# Given
63-
env = {"OTEL_EXPORTER_OTLP_ENDPOINT": "http://collector:4318"}
64-
mock_log_provider = MagicMock(spec=LoggerProvider)
65-
mock_tracer_provider = MagicMock(spec=TracerProvider)
66-
mock_otel_log_processor = MagicMock(spec=Processor)
66+
monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://collector:4318")
67+
68+
mock_log_provider = mocker.MagicMock(spec=LoggerProvider)
69+
mock_tracer_provider = mocker.MagicMock(spec=TracerProvider)
70+
mock_otel_log_processor = mocker.MagicMock(spec=Processor)
71+
72+
mock_setup = mocker.patch("common.core.main.setup_logging")
73+
mock_build_log = mocker.patch(
74+
"common.core.otel.build_otel_log_provider",
75+
return_value=mock_log_provider,
76+
)
77+
mock_build_tracer = mocker.patch(
78+
"common.core.otel.build_tracer_provider",
79+
return_value=mock_tracer_provider,
80+
)
81+
mock_make_processor = mocker.patch(
82+
"common.core.otel.make_structlog_otel_processor",
83+
return_value=mock_otel_log_processor,
84+
)
85+
mock_setup_tracing = mocker.patch("common.core.otel.setup_tracing")
6786

6887
# When
69-
with (
70-
patch.dict(os.environ, env),
71-
patch("common.core.main.setup_logging") as mock_setup,
72-
patch(
73-
"common.core.otel.build_otel_log_provider",
74-
return_value=mock_log_provider,
75-
) as mock_build_log,
76-
patch(
77-
"common.core.otel.build_tracer_provider",
78-
return_value=mock_tracer_provider,
79-
) as mock_build_tracer,
80-
patch(
81-
"common.core.otel.make_structlog_otel_processor",
82-
return_value=mock_otel_log_processor,
83-
) as mock_make_processor,
84-
patch("common.core.otel.setup_tracing") as mock_setup_tracing,
85-
ensure_cli_env(),
86-
):
88+
with ensure_cli_env():
8789
pass
8890

8991
# Then — OTel providers are built with the right endpoint and service name
@@ -106,31 +108,29 @@ def test_ensure_cli_env__otel_endpoint_set__configures_otel_processors() -> None
106108
]
107109

108110

109-
def test_ensure_cli_env__otel_custom_service_name_and_excluded_urls__passes_to_providers() -> (
110-
None
111-
):
111+
def test_ensure_cli_env__otel_custom_service_name_and_excluded_urls__passes_to_providers(
112+
monkeypatch: pytest.MonkeyPatch,
113+
mocker: MockerFixture,
114+
) -> None:
112115
# Given
113-
env = {
114-
"OTEL_EXPORTER_OTLP_ENDPOINT": "http://collector:4318",
115-
"OTEL_SERVICE_NAME": "my-service",
116-
"OTEL_TRACING_EXCLUDED_URL_PATHS": "health/liveness,health/readiness",
117-
}
116+
monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://collector:4318")
117+
monkeypatch.setenv("OTEL_SERVICE_NAME", "my-service")
118+
monkeypatch.setenv(
119+
"OTEL_TRACING_EXCLUDED_URL_PATHS", "health/liveness,health/readiness"
120+
)
121+
122+
mock_build_log = mocker.patch(
123+
"common.core.otel.build_otel_log_provider",
124+
return_value=mocker.MagicMock(spec=LoggerProvider),
125+
)
126+
mock_build_tracer = mocker.patch(
127+
"common.core.otel.build_tracer_provider",
128+
return_value=mocker.MagicMock(spec=TracerProvider),
129+
)
130+
mock_setup_tracing = mocker.patch("common.core.otel.setup_tracing")
118131

119132
# When
120-
with (
121-
patch.dict(os.environ, env),
122-
patch("common.core.main.setup_logging"),
123-
patch(
124-
"common.core.otel.build_otel_log_provider",
125-
return_value=MagicMock(spec=LoggerProvider),
126-
) as mock_build_log,
127-
patch(
128-
"common.core.otel.build_tracer_provider",
129-
return_value=MagicMock(spec=TracerProvider),
130-
) as mock_build_tracer,
131-
patch("common.core.otel.setup_tracing") as mock_setup_tracing,
132-
ensure_cli_env(),
133-
):
133+
with ensure_cli_env():
134134
pass
135135

136136
# Then
@@ -146,3 +146,25 @@ def test_ensure_cli_env__otel_custom_service_name_and_excluded_urls__passes_to_p
146146
mock_build_tracer.return_value,
147147
excluded_urls="health/liveness,health/readiness",
148148
)
149+
150+
151+
def test_ensure_cli_env__docgen_in_argv__sets_docgen_mode(
152+
monkeypatch: pytest.MonkeyPatch,
153+
) -> None:
154+
# Given
155+
monkeypatch.setattr("sys.argv", ["flagsmith", "docgen", "metrics"])
156+
157+
# When / Then
158+
with ensure_cli_env():
159+
assert os.environ.get("DOCGEN_MODE") == "true"
160+
161+
162+
def test_ensure_cli_env__task_processor_in_argv__sets_run_by_processor(
163+
monkeypatch: pytest.MonkeyPatch,
164+
) -> None:
165+
# Given
166+
monkeypatch.setattr("sys.argv", ["flagsmith", "task-processor"])
167+
168+
# When / Then
169+
with ensure_cli_env():
170+
assert os.environ.get("RUN_BY_PROCESSOR") == "true"

0 commit comments

Comments
 (0)