fix sandbox replace trace identity
This commit is contained in:
parent
dff28efecf
commit
02770bce7d
3 changed files with 120 additions and 7 deletions
38
tasks.md
38
tasks.md
|
|
@ -281,9 +281,45 @@
|
||||||
### M23. Boundary review для sandbox observability
|
### M23. Boundary review для sandbox observability
|
||||||
|
|
||||||
- Субагент: `code-reviewer`
|
- Субагент: `code-reviewer`
|
||||||
- Статус: pending
|
- Статус: in_progress
|
||||||
- Зависимости: `M22`
|
- Зависимости: `M22`
|
||||||
- Commit required: no
|
- Commit required: no
|
||||||
- Scope: проверить, что observability изменения закрывают issue #11 и FR-034 без нарушения clean architecture
|
- Scope: проверить, что observability изменения закрывают issue #11 и FR-034 без нарушения clean architecture
|
||||||
- Файлы: весь измененный код после `M19`-`M22`
|
- Файлы: весь измененный код после `M19`-`M22`
|
||||||
- Критерии приемки: inner layers не импортируют OTel; Docker-specific tracing остается в `adapter/docker/`; current-state и duration metrics достаточно покрывают sandbox lifecycle; замечания сведены к minor или отсутствуют
|
- Критерии приемки: inner layers не импортируют OTel; Docker-specific tracing остается в `adapter/docker/`; current-state и duration metrics достаточно покрывают sandbox lifecycle; замечания сведены к minor или отсутствуют
|
||||||
|
|
||||||
|
## Follow-up после M23 boundary review
|
||||||
|
|
||||||
|
### M24. Исправить replace trace identity в CreateSandbox
|
||||||
|
|
||||||
|
- Субагент: `feature-developer`
|
||||||
|
- Статус: completed
|
||||||
|
- Зависимости: `M23`
|
||||||
|
- Commit required: yes
|
||||||
|
- Commit message: `fix sandbox replace trace identity`
|
||||||
|
- Scope: устранить смешение old/new sandbox identifiers в replace path usecase tracing
|
||||||
|
- Файлы: `usecase/sandbox.py`, при необходимости точечные тесты в `test/*`
|
||||||
|
- Решение: сохранять старые и новые sandbox identifiers в отдельных span attrs или child spans так, чтобы replace success и replace failure оставались однозначно трассируемыми
|
||||||
|
- Критерии приемки: replace path не перетирает previous/new identifiers; при replace failure span остается консистентным и отражает обе стороны lifecycle
|
||||||
|
|
||||||
|
### M25. Добрать failure-path observability regression tests
|
||||||
|
|
||||||
|
- Субагент: `test-engineer`
|
||||||
|
- Статус: pending
|
||||||
|
- Зависимости: `M24`
|
||||||
|
- Commit required: yes
|
||||||
|
- Commit message: `add sandbox observability failure tests`
|
||||||
|
- Scope: покрыть tests для replace-failure trace, cleanup error metrics/spans и Docker stop observability
|
||||||
|
- Файлы: `test/test_sandbox_usecase.py`, `test/test_docker_runtime.py`, при необходимости другие focused tests в `test/*`
|
||||||
|
- Решение: использовать presence-based assertions и проверять ключевые span/metric contracts без brittle exact-order checks
|
||||||
|
- Критерии приемки: есть тест на replace failure tracing; есть тест на `sandbox.cleanup.error.total`; есть тесты на Docker stop observability для success/error/not_found или эквивалентного набора outcome paths
|
||||||
|
|
||||||
|
### M26. Повторный boundary review для sandbox observability
|
||||||
|
|
||||||
|
- Субагент: `code-reviewer`
|
||||||
|
- Статус: pending
|
||||||
|
- Зависимости: `M25`
|
||||||
|
- Commit required: no
|
||||||
|
- Scope: подтвердить, что follow-up fixes закрыли M23 замечания без новых boundary нарушений
|
||||||
|
- Файлы: весь измененный код после `M24`-`M25`
|
||||||
|
- Критерии приемки: нет замечаний по replace tracing identity и missing failure-path observability coverage; clean architecture по-прежнему соблюдена
|
||||||
|
|
|
||||||
|
|
@ -9,7 +9,7 @@ from adapter.observability.noop import NoopMetrics, NoopTracer
|
||||||
from domain.sandbox import SandboxSession, SandboxStatus
|
from domain.sandbox import SandboxSession, SandboxStatus
|
||||||
from repository.sandbox_lock import ProcessLocalSandboxLifecycleLocker
|
from repository.sandbox_lock import ProcessLocalSandboxLifecycleLocker
|
||||||
from repository.sandbox_session import InMemorySandboxSessionRepository
|
from repository.sandbox_session import InMemorySandboxSessionRepository
|
||||||
from usecase.interface import AttrValue, Attrs
|
from usecase.interface import Attrs, AttrValue
|
||||||
from usecase.sandbox import CleanupExpiredSandboxes, CreateSandbox, CreateSandboxCommand
|
from usecase.sandbox import CleanupExpiredSandboxes, CreateSandbox, CreateSandboxCommand
|
||||||
|
|
||||||
CHAT_ID = UUID('11111111-1111-1111-1111-111111111111')
|
CHAT_ID = UUID('11111111-1111-1111-1111-111111111111')
|
||||||
|
|
@ -486,6 +486,10 @@ def test_create_sandbox_replace_records_observability_and_final_active_count(
|
||||||
'usecase.create_sandbox',
|
'usecase.create_sandbox',
|
||||||
{'chat.id': str(CHAT_ID)},
|
{'chat.id': str(CHAT_ID)},
|
||||||
{
|
{
|
||||||
|
'sandbox.previous_session.id': str(SESSION_OLD_ID),
|
||||||
|
'sandbox.previous_container.id': 'container-old',
|
||||||
|
'sandbox.new_session.id': str(SESSION_NEW_ID),
|
||||||
|
'sandbox.new_container.id': f'container-{SESSION_NEW_ID}',
|
||||||
'session.id': str(SESSION_NEW_ID),
|
'session.id': str(SESSION_NEW_ID),
|
||||||
'container.id': f'container-{SESSION_NEW_ID}',
|
'container.id': f'container-{SESSION_NEW_ID}',
|
||||||
'sandbox.result': 'replaced',
|
'sandbox.result': 'replaced',
|
||||||
|
|
@ -649,6 +653,59 @@ def test_create_sandbox_error_records_observability(monkeypatch) -> None:
|
||||||
assert excinfo.value in span.errors
|
assert excinfo.value in span.errors
|
||||||
|
|
||||||
|
|
||||||
|
def test_create_sandbox_replace_stop_failure_preserves_separate_identities(
|
||||||
|
monkeypatch,
|
||||||
|
) -> None:
|
||||||
|
now = datetime(2026, 4, 2, 12, 0, tzinfo=UTC)
|
||||||
|
expired_session = SandboxSession(
|
||||||
|
session_id=SESSION_OLD_ID,
|
||||||
|
chat_id=CHAT_ID,
|
||||||
|
container_id='container-old',
|
||||||
|
status=SandboxStatus.RUNNING,
|
||||||
|
created_at=now - timedelta(minutes=10),
|
||||||
|
expires_at=now,
|
||||||
|
)
|
||||||
|
repository = InMemorySandboxSessionRepository()
|
||||||
|
repository.save(expired_session)
|
||||||
|
metrics = RecordingMetrics()
|
||||||
|
tracer = RecordingTracer()
|
||||||
|
usecase = CreateSandbox(
|
||||||
|
repository=repository,
|
||||||
|
locker=FakeLocker(),
|
||||||
|
runtime=FailingStopRuntime('container-old'),
|
||||||
|
clock=FakeClock(now),
|
||||||
|
logger=FakeLogger(),
|
||||||
|
metrics=metrics,
|
||||||
|
tracer=tracer,
|
||||||
|
ttl=timedelta(minutes=5),
|
||||||
|
)
|
||||||
|
monkeypatch.setattr('usecase.sandbox._new_session_id', lambda: SESSION_NEW_ID)
|
||||||
|
|
||||||
|
with pytest.raises(RuntimeError, match='stop_failed') as excinfo:
|
||||||
|
usecase.execute(CreateSandboxCommand(chat_id=CHAT_ID))
|
||||||
|
|
||||||
|
_assert_increment_metric_present(
|
||||||
|
metrics,
|
||||||
|
'sandbox.create.total',
|
||||||
|
attrs={'result': 'error'},
|
||||||
|
)
|
||||||
|
span = _find_span(
|
||||||
|
tracer,
|
||||||
|
'usecase.create_sandbox',
|
||||||
|
{'chat.id': str(CHAT_ID)},
|
||||||
|
{
|
||||||
|
'sandbox.previous_session.id': str(SESSION_OLD_ID),
|
||||||
|
'sandbox.previous_container.id': 'container-old',
|
||||||
|
'sandbox.new_session.id': str(SESSION_NEW_ID),
|
||||||
|
'sandbox.result': 'error',
|
||||||
|
},
|
||||||
|
)
|
||||||
|
assert 'sandbox.new_container.id' not in span.attrs
|
||||||
|
assert 'session.id' not in span.attrs
|
||||||
|
assert 'container.id' not in span.attrs
|
||||||
|
assert excinfo.value in span.errors
|
||||||
|
|
||||||
|
|
||||||
def test_create_sandbox_serializes_duplicate_concurrent_create_for_chat_id(
|
def test_create_sandbox_serializes_duplicate_concurrent_create_for_chat_id(
|
||||||
monkeypatch,
|
monkeypatch,
|
||||||
) -> None:
|
) -> None:
|
||||||
|
|
|
||||||
|
|
@ -67,10 +67,22 @@ class CreateSandbox:
|
||||||
return session
|
return session
|
||||||
|
|
||||||
result = 'created'
|
result = 'created'
|
||||||
|
new_session_id: UUID | None = None
|
||||||
if session is not None:
|
if session is not None:
|
||||||
result = 'replaced'
|
result = 'replaced'
|
||||||
span.set_attribute('session.id', str(session.session_id))
|
new_session_id = _new_session_id()
|
||||||
span.set_attribute('container.id', session.container_id)
|
span.set_attribute(
|
||||||
|
'sandbox.previous_session.id',
|
||||||
|
str(session.session_id),
|
||||||
|
)
|
||||||
|
span.set_attribute(
|
||||||
|
'sandbox.previous_container.id',
|
||||||
|
session.container_id,
|
||||||
|
)
|
||||||
|
span.set_attribute(
|
||||||
|
'sandbox.new_session.id',
|
||||||
|
str(new_session_id),
|
||||||
|
)
|
||||||
self._logger.info(
|
self._logger.info(
|
||||||
'sandbox_replaced',
|
'sandbox_replaced',
|
||||||
attrs=_sandbox_attrs(session),
|
attrs=_sandbox_attrs(session),
|
||||||
|
|
@ -81,16 +93,24 @@ class CreateSandbox:
|
||||||
|
|
||||||
created_at = self._clock.now()
|
created_at = self._clock.now()
|
||||||
expires_at = created_at + self._ttl
|
expires_at = created_at + self._ttl
|
||||||
session_id = _new_session_id()
|
if new_session_id is None:
|
||||||
span.set_attribute('session.id', str(session_id))
|
new_session_id = _new_session_id()
|
||||||
|
span.set_attribute('session.id', str(new_session_id))
|
||||||
new_session = self._runtime.create(
|
new_session = self._runtime.create(
|
||||||
session_id=session_id,
|
session_id=new_session_id,
|
||||||
chat_id=chat_id,
|
chat_id=chat_id,
|
||||||
created_at=created_at,
|
created_at=created_at,
|
||||||
expires_at=expires_at,
|
expires_at=expires_at,
|
||||||
)
|
)
|
||||||
|
if result == 'replaced':
|
||||||
|
span.set_attribute(
|
||||||
|
'sandbox.new_container.id',
|
||||||
|
new_session.container_id,
|
||||||
|
)
|
||||||
self._repository.save(new_session)
|
self._repository.save(new_session)
|
||||||
_set_active_count(self._metrics, self._repository)
|
_set_active_count(self._metrics, self._repository)
|
||||||
|
if result == 'replaced':
|
||||||
|
span.set_attribute('session.id', str(new_session.session_id))
|
||||||
span.set_attribute('container.id', new_session.container_id)
|
span.set_attribute('container.id', new_session.container_id)
|
||||||
span.set_attribute('sandbox.result', result)
|
span.set_attribute('sandbox.result', result)
|
||||||
self._metrics.increment(
|
self._metrics.increment(
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue