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][Spans] Instrument edxapp celery workers in Datadog #584

Closed
1 task
robrap opened this issue Mar 28, 2024 · 11 comments
Closed
1 task

[APM][Spans] Instrument edxapp celery workers in Datadog #584

robrap opened this issue Mar 28, 2024 · 11 comments
Assignees

Comments

@robrap
Copy link
Contributor

robrap commented Mar 28, 2024

See https://ddtrace.readthedocs.io/en/stable/integrations.html#celery.

I also asked in tech-external-datadog.

  • Required for ATAM decision: No (Assuming it is possible)
  • Required for contract pricing: Yes (To see load)
  • Required for completing migration: Yes

AC:

  • Instrument celery for workers (and potentially task producers)
@robrap robrap added this to Arch-BOM Mar 28, 2024
@robrap robrap converted this from a draft issue Mar 28, 2024
@robrap robrap changed the title Instrument Celery in DD Instrument Celery in Datadog Mar 28, 2024
@robrap robrap changed the title Instrument Celery in Datadog Instrument edxapp celery workers in Datadog Mar 29, 2024
@robrap robrap moved this to Groomed in Arch-BOM Mar 29, 2024
@robrap robrap changed the title Instrument edxapp celery workers in Datadog [APM][Spans] Instrument edxapp celery workers in Datadog Apr 9, 2024
@robrap robrap moved this from Groomed to Prioritized in Arch-BOM Apr 9, 2024
@robrap
Copy link
Contributor Author

robrap commented Apr 16, 2024

It's unclear whether the instrumentation is both for firing tasks (webapps) and consuming tasks (workers), or just for the workers.

@connorhaugh connorhaugh self-assigned this Apr 16, 2024
@connorhaugh
Copy link

  1. Check to see if arch-experiments is still installed/deployed. List all ides where celery workers are used.
  2. Figure out where we NEED to be pluggable, and where we don't need to be pluggable.
  3. Maybe use arch-experiments as a point for plugin-requiring code.
  4. Propose a plan to axim for implementation details, get buy in.

@connorhaugh
Copy link

  1. determine in NR if kafka is a "big enough thing" to matter now, and maybe do it alongside celery if it is trivial.

@robrap robrap moved this from Prioritized to In Progress in Arch-BOM Apr 16, 2024
@connorhaugh
Copy link

arch-experiments is still installed/deployed.

Yes. https://github.com/edx/edx-internal/blob/66d1c10e06e685369bf00947870073625f0fc6a4/ansible/vars/prod-edx.yml#L317

List all IDEs where celery workers are used.

  • Edx-Platform
  • Ecommerce (ecommerce-worker)
  • Credentials Service
  • Enterprise Catalog
  • Course Discovery
  • Enterprise Access
  • Registrar

@connorhaugh
Copy link

It's unclear whether the instrumentation is both for firing tasks (webapps) and consuming tasks (workers), or just for the workers.

From some digging, it seems that there is no clear documentation on what occurs. I think from what I see in the data the firing tasks are cataloged, I don't really know exactly what denotes a firing task though.

I was able to create logs for celery tasks for edx-platform in devstack, which is proof of concept enough for the other services (it took a while to set up local parallel celery on edx-platform and that is already a paved path).

Here are the PRs for the required services, followed by the revert PR.

edx platform:
openedx/edx-platform#34537

openedx/enterprise-catalog#817

openedx-unsupported/registrar#612

@connorhaugh
Copy link

@robrap
Copy link
Contributor Author

robrap commented Apr 22, 2024

@connorhaugh: Do we have a long-term plan, or were we waiting until after the experiment for this? I not well thought through plan might be:

  1. Introduce a setting for a pre-celery config hook.
  2. Introduce something like 2u-django-utils that can house code similar to edx-django-utils, but that we privately plug in to all our services, that would implement the pre-celery hooks api.

@robrap
Copy link
Contributor Author

robrap commented Apr 26, 2024

@timmc-edx has a PR that probably needs to land first, which may relate to the longer term version of this work.

@connorhaugh
Copy link

@timmc-edx
Copy link
Member

@connorhaugh This can be closed out now, right? (Along with the PR in the previous comment.)

@robrap
Copy link
Contributor Author

robrap commented Jun 7, 2024

@timmc-edx: FYI: I'm closing this Issue, and I closed the above PR. I assume at this point we do not need this.

@robrap robrap closed this as completed Jun 7, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Arch-BOM Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

3 participants