fix: set proper span status

[cristipufu]

No description provided.

[cristipufu]

/help review

[cristipufu]

/ai generated [2837508517]

Code Review for PR #82

1. File: samples/github-helper-agent/README.md

Changes:

• Modified the list of supported reviewer commands to add /help summarize:

  - Reviewer: `/help suggest`, `/help review`, `/help test`
  + Reviewer: `/help summarize`, `/help suggest`, `/help review`, `/help test`

Review:

• Good Practice: The documentation is kept up to date with the actual supported commands. This is important for user clarity and onboarding.
• No Issues: The change is simple, correct, and improves the documentation.

---

2. File: src/uipath_mcp/_cli/_runtime/_tracer.py

Changes:

• In two places, changed the span attribute from "error_message" to "error", and added span.set_status(StatusCode.ERROR) when handling errors:

  - span.set_attribute("error_message", root_value.error.message)
  + span.set_attribute("error", root_value.error.message)
  + span.set_status(StatusCode.ERROR)

• The same change is made in both the response and error handling branches.

Review:

• Correctness:
  • Setting the span status to StatusCode.ERROR is the correct way to mark a span as failed in OpenTelemetry, which improves observability and downstream error tracking.
  • Changing the attribute from "error_message" to "error" is a minor but reasonable change, possibly for consistency with conventions or other code.
• Consistency: The change is applied in both relevant code paths, ensuring consistent error reporting.
• Backward Compatibility: If any external systems or dashboards rely on the "error_message" attribute, this could be a breaking change. However, "error" is a more standard attribute name in many tracing systems.
• Code Quality: The code remains clean and readable. The logic is clear and the error handling is explicit.

Suggestions:

• If there are any dashboards, alerting rules, or documentation that reference the "error_message" attribute, ensure they are updated to use "error" instead.
• Consider adding or updating tests to verify that spans are now correctly marked with StatusCode.ERROR and the "error" attribute.

---

Summary

• Documentation: Updated to reflect the new /help summarize command.
• Tracing/Error Reporting: Improved by setting the proper span status and using a more standard error attribute.

No major issues found. The changes are correct, improve observability, and keep documentation up to date.

Recommendation: Approve after verifying any dependencies on the old "error_message" attribute.

[cristipufu]

/help test

[cristipufu]

/ai generated [2837515394]

Based on the changes in this PR, here are suggested tests to ensure the modifications work as intended:

---

1. For src/uipath_mcp/_cli/_runtime/_tracer.py

a. Test: Span Status and Error Attribute on Error

Objective:
Verify that when an error occurs, the span is marked with StatusCode.ERROR and the "error" attribute is set with the correct error message.

Test Steps:

1. Mock or simulate a function that triggers the error handling code path in _tracer.py.
2. Use a test OpenTelemetry tracer provider to capture spans.
3. Trigger the error scenario.
4. Assert that:
   • The resulting span has status.code == StatusCode.ERROR.
   • The span has an attribute "error" with the expected error message.
   • The span does not have an attribute "error_message".

Example (pytest + opentelemetry-sdk):

from opentelemetry.sdk.trace import TracerProvider
from opentelemetry.sdk.trace.export import InMemorySpanExporter, SimpleSpanProcessor
from opentelemetry.trace.status import StatusCode

def test_span_error_status_and_attribute(monkeypatch):
    # Setup in-memory exporter and tracer
    exporter = InMemorySpanExporter()
    provider = TracerProvider()
    provider.add_span_processor(SimpleSpanProcessor(exporter))
    tracer = provider.get_tracer(__name__)

    # Patch the tracer in your module to use this one
    monkeypatch.setattr("uipath_mcp._cli._runtime._tracer.tracer", tracer)

    # Simulate error scenario (call the function that triggers the error)
    # ... (invoke the relevant function here)

    spans = exporter.get_finished_spans()
    assert spans, "No spans were exported"
    span = spans[0]
    assert span.status.status_code == StatusCode.ERROR
    assert "error" in span.attributes
    assert "error_message" not in span.attributes

---

b. Test: Span Status on Success

Objective:
Verify that when no error occurs, the span is not marked as error and does not have the "error" attribute.

Test Steps:

1. Simulate a successful execution path.
2. Assert that:
   • The span status is not StatusCode.ERROR.
   • The "error" attribute is not present.

---

2. For samples/github-helper-agent/README.md

No code tests required for documentation changes, but you may want to lint or check the markdown rendering if you have automated docs tests.

---

3. Regression Test

Objective:
Ensure that any systems or logs that previously relied on "error_message" are not broken, or update them accordingly.

---

Summary

• Add/modify tests to check for the new "error" attribute and StatusCode.ERROR on spans when errors occur.
• Ensure no "error_message" attribute is set.
• No tests needed for the README change.

Let me know if you need example test code for a specific framework or more details!