-
Notifications
You must be signed in to change notification settings - Fork 18
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
feat: Add run job traces #292
Conversation
9a90017
to
1edaf9b
Compare
17c73f2
to
bd89a0d
Compare
Co-authored-by: logan <[email protected]>
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.
Looks good and runs locally! 🚀
|
||
- name: Display resource provider logs | ||
run: docker logs resource-provider | ||
|
||
- name: Display solver logs | ||
run: docker logs solver | ||
|
||
- name: Display chain logs | ||
run: docker logs chain |
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.
what's the purpose / value of having all these logs in the integration tests?
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.
It's to help debug when the integration tests fail. While working on this pull request, there was an issue where there were failures in the runner environment only. Had to add logs to get to the bottom of it.
I think there's no harm in printing the logs? Makes it easier to see what may have failed.
"github.com/rs/zerolog/log" | ||
) | ||
|
||
func GetGPUInfo() []string { |
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.
this will be great to have for cross-referencing 🙏
ce7096d
to
215e802
Compare
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @narbs91 on file. In order for us to review and merge your code, please open a new pull request and sign the Contributor License Agreement following the instructions in our contributing guide. Once the pull request is merged, ping a maintainer who will summon me again. |
Review Type Requested (choose one):
Summary
This pull request implements the following changes:
--disable-telemetry
CLI flagtelemetry_url
configtelemetry_token
configGetOTelServiceName
helper function and table testVersion
to system package to make it accessible as a resource attributetest
GitHub action workflowTask/Issue reference
Closes: #293
Details
This pull request adds our initial tracing implementation with a single trace over resource provider job runs.
We collect the following information about resource providers:
dev
,devnet
, ortestnet
)This information will also be collected for any service where we add tracing.
We collect the following information about jobs:
The public address, job offer ID, deal ID, trace ID logs are identifiers for looking up traces.
How to test this code?
Run our forthcoming observability implementation (https://github.com/Lilypad-Tech/observability/tree/dev-ronelcanaria/feat-add-alloy):
Navigate to the Grafana Dashboard at
http://localhost:3000
. Start the full Lilypad stack, then run a job.The resource provider should send the trace to the observability stack where it can be viewed in
Home > Explore
, select "Tempo" at the top, and search. It's possible to filter onrun_job
for the span name, but shouldn't be needed because we only have one trace.