Repository navigation
fix: correct terminal telemetry status and timing #865
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
26536b6
82fc876
20cd59b
3d77e65
352a137
28338a6
ef406b5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -892,16 +892,25 @@ def on_operation_end(self, info: OperationEndInfo) -> None: | |
| span_id_override=span_id_override, | ||
| ) | ||
|
|
||
| end_timestamp = to_otel_timestamp( | ||
| end_time or datetime.datetime.now(datetime.UTC) | ||
| ) | ||
|
zhongkechen marked this conversation as resolved.
Resolved
|
||
| if info.error: | ||
| span.set_status(StatusCode.ERROR, info.error.message or "") | ||
| record_error_object_exception(span, info.error) | ||
| record_error_object_exception(span, info.error, timestamp=end_timestamp) | ||
| elif info.status in { | ||
| OperationStatus.FAILED, | ||
| OperationStatus.TIMED_OUT, | ||
| OperationStatus.STOPPED, | ||
| }: | ||
| span.set_status(StatusCode.ERROR) | ||
|
Comment on lines
+901
to
+906
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both views now contain the same three-branch status mapping. PR #752: correct exported status mappings changed it in both views, and this PR changes both again. JS keeps the mapping in one function, _FAILURE_STATUSES = frozenset(
{OperationStatus.FAILED, OperationStatus.TIMED_OUT, OperationStatus.STOPPED}
)
def set_operation_span_status(span: Span, info: OperationEndInfo) -> None:
if info.error:
span.set_status(StatusCode.ERROR, info.error.message or "")
elif info.status in _FAILURE_STATUSES:
span.set_status(StatusCode.ERROR)
elif info.status is OperationStatus.SUCCEEDED:
span.set_status(StatusCode.OK) |
||
| elif info.status is OperationStatus.SUCCEEDED: | ||
| span.set_status(StatusCode.OK) | ||
|
|
||
| self._note_parent_end(info.parent_id, end_time) | ||
| popped = self._pop_span(info.operation_id) | ||
| if popped is not None: | ||
| popped.end(end_time=_to_otel_timestamp(end_time)) | ||
| popped.end(end_time=end_timestamp) | ||
|
|
||
| def _start_span( | ||
| self, | ||
|
|
@@ -1024,18 +1033,20 @@ def on_user_function_end(self, info: UserFunctionEndInfo) -> None: | |
| and info.outcome is not UserFunctionOutcome.INCOMPLETE | ||
| ): | ||
| span.set_attributes(self._operation_attributes(info)) | ||
| end_timestamp = to_otel_timestamp( | ||
| end_time or datetime.datetime.now(datetime.UTC) | ||
| ) | ||
| if info.outcome is UserFunctionOutcome.FAILED: | ||
| span.set_status( | ||
| StatusCode.ERROR, info.error.message if info.error else "" | ||
| ) | ||
| record_error_object_exception(span, info.error) | ||
| record_error_object_exception(span, info.error, timestamp=end_timestamp) | ||
| else: | ||
| span.set_status(StatusCode.OK) | ||
|
|
||
| end_time = ensure_end_after_start(info.start_time, info.end_time) | ||
| popped = self._pop_span(key) | ||
| if popped is not None: | ||
| popped.end(end_time=_to_otel_timestamp(end_time)) | ||
| popped.end(end_time=end_timestamp) | ||
|
|
||
| # Restore the enclosing context by releasing the scope this user | ||
| # function attached, so the parent operation (or, at the top level, the | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,134 @@ | ||
| """Caught operation failures preserve telemetry through real suspension/replay.""" | ||
|
|
||
| import json | ||
| from typing import Any | ||
|
|
||
| import pytest | ||
| from aws_durable_execution_sdk_python import DurableContext, durable_execution | ||
| from aws_durable_execution_sdk_python.config import ChildConfig, Duration | ||
| from aws_durable_execution_sdk_python.exceptions import ChildContextError, InvokeError | ||
| from aws_durable_execution_sdk_python.plugin import ( | ||
| DurableInstrumentationPlugin, | ||
| InvocationStartInfo, | ||
| OperationEndInfo, | ||
| ) | ||
| from aws_durable_execution_sdk_python_testing.runner import DurableFunctionTestRunner | ||
| from opentelemetry.sdk.trace import TracerProvider | ||
| from opentelemetry.sdk.trace.export import SimpleSpanProcessor | ||
| from opentelemetry.sdk.trace.export.in_memory_span_exporter import InMemorySpanExporter | ||
| from opentelemetry.trace import StatusCode | ||
|
|
||
| from aws_durable_execution_sdk_python_otel import ( | ||
| ExecutionOtelPluginFactory, | ||
| InvocationOtelPluginFactory, | ||
| OtelPluginConfig, | ||
| ) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "view", [ExecutionOtelPluginFactory, InvocationOtelPluginFactory] | ||
| ) | ||
| @pytest.mark.parametrize("failure_kind", ["child", "virtual", "invoke"]) | ||
| @pytest.mark.parametrize("resume", [False, True]) | ||
| def test_caught_failure_keeps_events_bounded_and_workflow_successful( | ||
| view: type[ExecutionOtelPluginFactory] | type[InvocationOtelPluginFactory], | ||
| failure_kind: str, | ||
| resume: bool, | ||
| monkeypatch: pytest.MonkeyPatch, | ||
| ) -> None: | ||
| monkeypatch.delenv("DURABLE_EXECUTION_PLUGINS", raising=False) | ||
| provider = TracerProvider() | ||
| exporter = InMemorySpanExporter() | ||
| provider.add_span_processor(SimpleSpanProcessor(exporter)) | ||
| factory = view( | ||
| OtelPluginConfig( | ||
| tracer_provider=provider, | ||
| enrich_logger=False, | ||
| context_extractor=lambda _: None, | ||
| ) | ||
| ) | ||
| saved_calls: list[str] = [] | ||
| failure_calls: list[str] = [] | ||
| observations: list[OperationEndInfo] = [] | ||
|
|
||
| class Observe(DurableInstrumentationPlugin): | ||
| def on_operation_end(self, info: OperationEndInfo) -> None: | ||
| if info.name == "caught-failure": | ||
| observations.append(info) | ||
|
|
||
| class ObserveFactory: | ||
| def create_plugin(self, _info: InvocationStartInfo) -> Observe: | ||
| return Observe() | ||
|
|
||
| def saved_step(_context: Any) -> str: | ||
| saved_calls.append("saved") | ||
| return "saved" | ||
|
|
||
| def failing_child(_context: DurableContext) -> str: | ||
| failure_calls.append("failed") | ||
| raise ValueError("actual child failure") | ||
|
|
||
| def failing_function(_event: Any, _context: Any) -> None: | ||
| failure_calls.append("failed") | ||
| raise ValueError("actual invoke failure") | ||
|
|
||
| def body(_event: Any, context: DurableContext) -> str: | ||
| saved = context.step(saved_step, name="saved-step") | ||
| try: | ||
| if failure_kind == "invoke": | ||
| context.invoke("failing-target", {}, name="caught-failure") | ||
| else: | ||
| context.run_in_child_context( | ||
| failing_child, | ||
| name="caught-failure", | ||
| config=ChildConfig(is_virtual=failure_kind == "virtual"), | ||
| ) | ||
| except (ChildContextError, InvokeError): | ||
| if resume: | ||
| context.wait(Duration.from_seconds(1), name="replay-wait") | ||
| return saved | ||
| raise AssertionError("Expected the original operation failure") | ||
|
|
||
| try: | ||
| with DurableFunctionTestRunner( | ||
| handler=durable_execution(body, plugins=[factory, ObserveFactory()]), | ||
| poll_interval=0.01, | ||
| ) as runner: | ||
| runner.register_function("failing-target", failing_function) | ||
| result = runner.run(input="{}", execution_timeout=15) | ||
| assert result.status.value == "SUCCEEDED" | ||
| assert result.result is not None and json.loads(result.result) == "saved" | ||
| assert saved_calls == ["saved"] | ||
| # Virtual contexts intentionally execute on replay; checkpointed child | ||
| # failures and invoke outcomes must return their stored failure instead. | ||
| assert len(failure_calls) == (2 if resume and failure_kind == "virtual" else 1) | ||
| assert observations | ||
| if failure_kind == "virtual": | ||
| assert all(info.start_time is None for info in observations) | ||
| spans = exporter.get_finished_spans() | ||
| failed = [ | ||
| span | ||
| for span in spans | ||
| if span.name == "caught-failure" | ||
| and span.attributes is not None | ||
| and span.attributes.get("durable.operation.status") == "FAILED" | ||
| ] | ||
| assert failed | ||
| for span in failed: | ||
| assert span.status.status_code is StatusCode.ERROR | ||
| (event,) = span.events | ||
| assert span.start_time is not None and span.end_time is not None | ||
| assert span.start_time <= event.timestamp == span.end_time | ||
| assert event.attributes is not None | ||
| assert event.attributes["exception.message"] in { | ||
| "actual child failure", | ||
| "actual invoke failure", | ||
| } | ||
| invocations = [span for span in spans if span.name == "Invocation"] | ||
| # An invoke can add its own suspension before the explicit replay wait. | ||
| assert len(invocations) >= (2 if resume else 1) | ||
| assert all(span.status.status_code is StatusCode.OK for span in invocations) | ||
| (workflow,) = [span for span in spans if span.name == "Workflow"] | ||
| assert workflow.status.status_code is StatusCode.OK | ||
| finally: | ||
| provider.shutdown() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The paragraph uses "normalized" and "invocation-local timestamp ordering" without defining them. The checkpointless sentence also does not match the runner. In Execution view, a virtual child that sleeps 50 ms exports a 50 ms span. So its span starts when the user function starts, not at completion. Can the paragraph state the rules directly and drop that sentence? For example: