Skip to content
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

feat: Close anomalous spans in Datadog middleware #798

Merged
merged 2 commits into from
Sep 19, 2024
Merged

Conversation

timmc-edx
Copy link
Member

Also, add an additional assertion in run_middleware to ensure that the middleware hasn't silently entered an error state. (Helped me debug a case where I had forgotten to fake a Waffle flag.)

Merge checklist:
Check off if complete or not applicable:

  • Version bumped
  • Changelog record added
  • Documentation updated (not only docstrings)
  • Fixup commits are squashed away
  • Unit tests added/updated
  • Manual testing instructions provided
  • Noted any: Concerns, dependencies, migration issues, deadlines, tickets

Also, add an additional assertion in `run_middleware` to ensure that the
middleware hasn't silently entered an error state. (Helped me debug a case
where I had forgotten to fake a Waffle flag.)
@timmc-edx
Copy link
Member Author

timmc-edx commented Sep 17, 2024

I wasn't able to do a completely faithful manual test because we don't have a way of generating the anomalous spans. However, I was able to do something sort of close by mangling the code a bit:

  1. Get the middleware installed and the feature activated in devstack
  2. Run LMS without DD_DJANGO_INSTRUMENT_MIDDLEWARE=false to ensure there's a fat stack of spans to work with, and DD_TRACE_DEBUG=true to confirm that spans really are being closed
  3. Modify close_anomalous_spans() to skip the duration check, use while walk_span.resource != 'edx_django_utils.monitoring.DeploymentMonitoringMiddleware.__call__': rather than looking for a span with name django.request, and print the current span at the end.

With that, it appears that the middleware above the arbitrarily selected one are finished but the selected one and the ones below are not (until the trace finishes naturally).

@timmc-edx timmc-edx merged commit 8e2868a into main Sep 19, 2024
6 of 7 checks passed
@timmc-edx timmc-edx deleted the timmc/close-spans branch September 19, 2024 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants