-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add Tracing and rework cli so that the controller command does not start a FastAPI app #669
Conversation
keithralphs
commented
Oct 14, 2024
- Add tracing intialisation and decorators
- In cli.py move some imports within functions to prevent unnecessary FastAPI apps being created
- Add capability to specify a config file in debug launchers
- Add Environment vars to initialise traceability options and set Jager export to off by default
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #669 +/- ##
==========================================
- Coverage 92.46% 92.35% -0.11%
==========================================
Files 35 35
Lines 1658 1793 +135
==========================================
+ Hits 1533 1656 +123
- Misses 125 137 +12 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to write tests such that we can check that the spans etc. we expect are being requested?
src/blueapi/worker/task_worker.py
Outdated
self._current_task_otel_context = get_trace_context() | ||
add_span_attributes({"next_task.task_id": next_task.task_id}) | ||
|
||
self._current.is_pending = False | ||
self._current.task.do_task(self._ctx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are now enough pieces of context that the worker needs to keep track of that it makes sense to box them up into a dataclass or similar object. I think that's a separate issue though...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please could you make issues to streamline the output from the CLI tool and to speed up the tests! The latter is especially important!
In Denmark, and unable to contest