Feat dynatrace backend - #46
Conversation
📝 WalkthroughWalkthroughAdds a new file ChangesDynatrace Backend
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/opentelemetry_mcp/backends/dynatrace.py (1)
180-181: ⚡ Quick winAdd explicit
return Noneafter logging the parse error.The implicit
Nonereturn is correct but explicit is clearer and ensures MyPy strict mode compliance.Proposed fix
except Exception as e: logger.error(f"Error parsing Dynatrace trace structural metrics: {e}") + return None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/opentelemetry_mcp/backends/dynatrace.py` around lines 180 - 181, In the exception handler for parsing Dynatrace trace structural metrics (the except Exception as e block), add an explicit `return None` statement immediately after the logger.error call. This makes the function's return behavior explicit and ensures compliance with MyPy strict mode, even though Python's implicit None return would work the same way.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/opentelemetry_mcp/backends/dynatrace.py`:
- Around line 171-179: The code assumes spans[0] is the root span, but the root
span is actually the one where parent_span_id is None. To fix this, locate the
root span by iterating through the spans list and finding the span where
parent_span_id equals None, then use that span's service_name and operation_name
for the TraceData construction instead of directly accessing
spans[0].service_name and spans[0].operation_name. This ensures correct values
regardless of the order Dynatrace returns spans in.
- Around line 166-169: The trace_duration calculation is incorrect because it
sums all span durations, which overcounts when spans execute concurrently and
overlap. Replace the sum-based calculation with the correct approach: calculate
the maximum end time across all spans (by finding max of start_time plus
duration_ms for each span), then subtract the minimum start time (trace_start)
to get the actual trace duration that accounts for parallel execution.
- Around line 96-104: The get_service_operations method constructs a URL by
directly interpolating the service_name parameter without URL encoding. When
service names contain special characters like /, %, or spaces, this produces
malformed requests. Import quote from urllib.parse at the top of the file, then
in the get_service_operations method, wrap the service_name parameter with
quote() when constructing the API endpoint URL in the self.client.get() call to
properly encode any special characters in the path segment.
- Line 155: The datetime.fromtimestamp() call in the trace data initialization
is creating a naive datetime in local time, which can cause timezone
inconsistencies when trace data is compared across systems. Modify the
datetime.fromtimestamp() call to explicitly specify UTC timezone by adding the
timezone parameter, ensuring the datetime object is timezone-aware and
consistently represents UTC time regardless of the system's local timezone.
- Around line 31-49: The search_traces method in the Dynatrace backend is
calling a non-existent API endpoint and using incompatible query parameters.
Replace the `/api/v2/traces` endpoint call with the correct Logs and Events API
endpoint that Dynatrace provides for querying span data. Instead of using
query.to_backend_params() which produces Jaeger-style parameters (service,
operation, minDuration), convert the TraceQuery object into a proper Dynatrace
Query Language (DQL) query string that reflects the actual filter criteria from
the TraceQuery object. Update the response parsing logic to handle the actual
response format from the Logs and Events API for span data, which will differ
from the current trace structure expectation.
---
Nitpick comments:
In `@src/opentelemetry_mcp/backends/dynatrace.py`:
- Around line 180-181: In the exception handler for parsing Dynatrace trace
structural metrics (the except Exception as e block), add an explicit `return
None` statement immediately after the logger.error call. This makes the
function's return behavior explicit and ensures compliance with MyPy strict
mode, even though Python's implicit None return would work the same way.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e118c821-c83f-4db5-92bb-d3943bdf436b
📒 Files selected for processing (1)
src/opentelemetry_mcp/backends/dynatrace.py
|
Hi, just following up on this PR whenever you get a chance to review it. Let me know if anything else is needed from my side. Thanks! |
Description
Closes #5
This PR adds support for Dynatrace as an observability backend, allowing the MCP server to query OpenTelemetry traces from Dynatrace environments.
Changes Made
DynatraceBackendclass insrc/opentelemetry_mcp/backends/dynatrace.pyimplementing theBaseBackendinterface.tests/backends/test_dynatrace.pyto cover core backend functionality and validation.Testing Done
Summary by CodeRabbit
Release Notes