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

DataDog support for Worker and Scheduler #1712

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from
Open

Conversation

aweiland
Copy link
Contributor

@aweiland aweiland commented Jan 20, 2025

  • New entrypoints for async tasks
  • Adding Datadog to worker and scheduler

📝 Description

🔗 Jira Ticket M2-8041

Changes include:

  • Added Datadog monkey patches
  • Added structlog for structured logging

🪤 Peer Testing

  • Ensure application starts up
  • Ensure logs are structured as expected

✏️ Notes

Copy link

github-actions bot commented Jan 21, 2025

➡️ Preview environment created: Click Me!

Copy link

❌ E2E tests failed

Copy link
Contributor

@sultanofcardio sultanofcardio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aweiland I have a few cosmetic suggestions, but I tested and it looks like this works 👍

docker-compose.yaml Outdated Show resolved Hide resolved
docker-compose.yaml Outdated Show resolved Hide resolved
logger.setLevel(logging.INFO)
logger.addHandler(logging.StreamHandler())
logger.info("Enabling Datadog")
# import ddtrace.auto # noqa
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# import ddtrace.auto # noqa

src/infrastructure/dependency/structured_logs.py Outdated Show resolved Hide resolved
# network={"client": {"ip": real_host, "port": client_port}},
# duration=process_time,
# )
# response.headers["X-Process-Time"] = str(process_time / 10 ** 9)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be putting this back in?

Suggested change
# response.headers["X-Process-Time"] = str(process_time / 10 ** 9)
response.headers["X-Process-Time"] = str(process_time / 10 ** 9)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I debated whether it's worth sending back to the client or not. We have this information in Datadog so it didn't seem useful to me.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, we can remove the comment

src/infrastructure/dependency/structured_logs.py Outdated Show resolved Hide resolved
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