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

move tracer to skyvern init #660

Merged
merged 1 commit into from
Jul 30, 2024
Merged

Conversation

wintonzheng
Copy link
Contributor

@wintonzheng wintonzheng commented Jul 30, 2024

🚀 This description was created by Ellipsis for commit 5a1b01f

Summary:

Moved ddtrace tracer configuration to skyvern/__init__.py for centralized initialization.

Key points:

  • Moved ddtrace tracer configuration from skyvern/forge/app.py to skyvern/__init__.py.
  • Ensures tracer is configured during skyvern module initialization.
  • Removed redundant tracer configuration from skyvern/forge/app.py.
  • Affects skyvern/__init__.py and skyvern/forge/app.py.

Generated with ❤️ by ellipsis.dev

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 5a1b01f in 31 seconds

More details
  • Looked at 47 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. skyvern/__init__.py:6
  • Draft comment:
    Moving the tracer configuration to __init__.py is a good practice for centralizing setup logic and ensuring it's executed before other imports. This helps in maintaining cleaner code and avoiding potential issues with the order of imports.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The PR moves the tracer configuration from skyvern/forge/app.py to skyvern/__init__.py. This centralizes the tracer setup, which is a common practice for better maintainability and to ensure that the tracer is configured before any other imports that might use it. This change seems appropriate as it avoids potential issues where the tracer might be used before being configured if other modules are loaded before app.py. The use of FilterRequestsOnUrl to filter out specific URLs (like heartbeat checks) from tracing is also a good practice to reduce unnecessary logging and overhead.

Workflow ID: wflow_SvvWaQhy0Il5ZfSh


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@wintonzheng wintonzheng merged commit b2d692f into main Jul 30, 2024
2 checks passed
@wintonzheng wintonzheng deleted the shu/move_tracer_to_skyvern_init branch July 30, 2024 23:13
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.

1 participant