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

[APM][Tags] Configure Datadog request resource names #620

Closed
6 tasks done
timmc-edx opened this issue May 2, 2024 · 2 comments
Closed
6 tasks done

[APM][Tags] Configure Datadog request resource names #620

timmc-edx opened this issue May 2, 2024 · 2 comments
Assignees

Comments

@timmc-edx
Copy link
Member

timmc-edx commented May 2, 2024

We need to settle on a resource_name format for Python APM. This has implications for trace metrics, browsing the UI, and monitoring.

As of 2024-05-15 we're using a handler-only format but we need to make a final decision.

A/C:

Resources

This is applied to the root span as the resource_name tag. HTTP method and urlpattern are also available on the root span as custom span tags http.method and http.route but there doesn't seem to be any other way to get the handler info.

Pros and cons

  • urlpattern format:
    • Available in default config (always accompanied by HTTP method).
    • Can be pretty simple like ^/ (for the home page) or really nasty like ^courses/(?P<course_id>[^/+]+(/|\+)[^/+]+(/|\+)[^/?]+)/xblock/(?P<usage_id>(?:i4x://?[^/]+/[^/]+/[^/]+/[^@]+(?:@[^/]+)?)|(?:[^/]+))/handler/(?P<handler>[^/]*)(?:/(?P<suffix>.*))?$ (xblock_handler).
    • Really bad for querying and for dashboards.
  • handler format:
    • Available with or without HTTP method using DD_DJANGO_USE_HANDLER_RESOURCE_FORMAT=true or DD_DJANGO_USE_LEGACY_RESOURCE_FORMAT=true respectively.
    • Can be a dotted module path to a function, e.g. lms.djangoapps.branding.views.index (the view function index in the module lms.djangoapps.branding.views) or to a class, e.g. enterprise.api.v1.views.enterprise_customer:EnterpriseCustomerViewSet.
    • Problem: View classes can have any number of methods, and Datadog doesn't tell us which one was invoked. EnterpriseCustomerViewSet has list, retrieve, dashboard_list, enroll_learners_in_courses, basic_list, toggle_universal_link, and partial_update. These can generally be distinguished by method + urlpattern, but not by handler class alone.
      • While we could make up for this by adding our own custom span tag in a new middleware, it wouldn't be available for certain kinds of monitors.
      • New Relic gives the full name, e.g. WebTransaction/Function/enterprise.api.v1.views.enterprise_customer:EnterpriseCustomerViewSet.enroll_learners_in_courses
      • Private ticket with Datadog regarding lack of complete Django viewfunc information: https://help.datadoghq.com/hc/en-us/requests/1694229
  • method in format:
    • Available with either handler or urlpattern.
    • If used with handler, would require parsing to get just the handler out of the resource name.
    • Helps disambiguate views when DD elides the full viewfunc.

Decision

  • We will switch to the DD_DJANGO_USE_HANDLER_RESOURCE_FORMAT=true resource name format (method + handler):
    • urlpattern resource names are just too hard to read, query, and pronounce for (for some of our IDAs and views), so we definitely want a handler-based format.
    • The only upside of urlpattern is being able to discriminate between different views in a viewset. We can capture some of that by including method in the resource name; this will help when the view functions are just read, create, delete, etc. It does not help when the view functions are distinct operations such as retirement_queue and cleanup.
    • Teams with more complicated viewsets will be able to use Trace Analytics in their monitors (using @http.route, @django.view, and similar tags) to be more specific than the resource name. They will have less granularity in the built-in dashboards and in certain other features.
  • We will provide a new middleware feature for IDAs that will add a tag on Django service entry spans that includes the full view function name. This will make Trace Analytics and dashboard queries more uniform and pleasant to write, rather than relying on http.route and django.view.
  • We will ask Datadog to fill in this missing info via a new flag. If/when they do this, teams will have a choice of whether to adopt it (except for LMS, which would just have this feature enabled, and teams would just have to adjust their boards and runbooks).
@timmc-edx timmc-edx converted this from a draft issue May 2, 2024
@timmc-edx timmc-edx self-assigned this May 2, 2024
timmc-edx added a commit to openedx-unsupported/configuration that referenced this issue May 3, 2024
This switches us from method+urlpattern resource naming (e.g. `GET ^/`)
to using handler (e.g. `lms.djangoapps.branding.views.index`).

This is applied to the root span as the `resource_name` tag. As method and
urlpattern are available on the root span as `http.method` and `http.route`
but there doesn't seem to be any other way to get the handler info, this is
our only option for ensuring that the handler will be available as a tag
(without having to parse after the fact or write more custom code.)

Part of edx/edx-arch-experiments#620
timmc-edx added a commit to openedx-unsupported/configuration that referenced this issue May 3, 2024
This switches us from method+urlpattern resource naming (e.g. `GET ^/`)
to using handler (e.g. `lms.djangoapps.branding.views.index`).

This is applied to the root span as the `resource_name` tag. As method and
urlpattern are available on the root span as `http.method` and `http.route`
but there doesn't seem to be any other way to get the handler info, this is
our only option for ensuring that the handler will be available as a tag
(without having to parse after the fact or write more custom code.)

Part of edx/edx-arch-experiments#620
timmc-edx added a commit to openedx-unsupported/configuration that referenced this issue May 6, 2024
This switches us from method+urlpattern resource naming (e.g. `GET ^/`)
to using handler (e.g. `lms.djangoapps.branding.views.index`).

This is applied to the root span as the `resource_name` tag. As method and
urlpattern are available on the root span as `http.method` and `http.route`
but there doesn't seem to be any other way to get the handler info, this is
our only option for ensuring that the handler will be available as a tag
(without having to parse after the fact or write more custom code.)

Part of edx/edx-arch-experiments#620
@timmc-edx
Copy link
Member Author

We'll use the spreadsheet to ensure deployments happen.

@github-project-automation github-project-automation bot moved this from In Progress to Done in Arch-BOM May 7, 2024
@timmc-edx timmc-edx reopened this May 15, 2024
@timmc-edx timmc-edx moved this from Done to In Progress in Arch-BOM May 15, 2024
@timmc-edx timmc-edx changed the title Configure Datadog request resource names [APM][Tags] Configure Datadog request resource names May 15, 2024
timmc-edx added a commit to edx/configuration that referenced this issue May 16, 2024
Replaces all instances of `DD_DJANGO_USE_LEGACY_RESOURCE_FORMAT` with
`DD_DJANGO_USE_HANDLER_RESOURCE_FORMAT`. Now resource names will be
prefixed with the HTTP method, like
`GET lms.djangoapps.branding.views.index`.

This is not ideal and loses us some precision in the resource name but
gives us something more readable, writable, and pronounceable. See
edx/edx-arch-experiments#620 for more background.
timmc-edx added a commit to edx/configuration that referenced this issue May 20, 2024
Replaces all instances of `DD_DJANGO_USE_LEGACY_RESOURCE_FORMAT` with
`DD_DJANGO_USE_HANDLER_RESOURCE_FORMAT`. Now resource names will be
prefixed with the HTTP method, like
`GET lms.djangoapps.branding.views.index`.

This is not ideal and loses us some precision in the resource name but
gives us something more readable, writable, and pronounceable. See
edx/edx-arch-experiments#620 for more background.
@github-project-automation github-project-automation bot moved this from In Progress to Done in Arch-BOM May 20, 2024
@timmc-edx
Copy link
Member Author

Verified in edxapp-lms and demographics in stage.

@jristau1984 jristau1984 moved this from Done to Done - Long Term Storage in Arch-BOM Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done - Long Term Storage
Development

No branches or pull requests

1 participant