Skip to content

improve tracing coverage #131

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

Merged
merged 13 commits into from
Jul 17, 2024
Merged

improve tracing coverage #131

merged 13 commits into from
Jul 17, 2024

Conversation

cctdaniel
Copy link
Contributor

@cctdaniel cctdaniel commented Jul 12, 2024

This PR enhances tracing by introducing the #[instrument] macro to more functions and adding additional fields to the current span in get_product.rs, subscribe_price.rs, and subscribe_price_sched.rs. This approach is async safe as it maintains the reference to the current span without manual enter/exit, ensuring the tracing context is preserved across await points. We should avoid manual enter/exit tracing logs already handled by #[instrument], and any custom spans should be cautious of this behavior to avoid manual span entry.

@cctdaniel cctdaniel changed the base branch from main to push-zrxvuuvnxrqt July 12, 2024 05:13
@cctdaniel cctdaniel changed the base branch from push-zrxvuuvnxrqt to main July 16, 2024 08:06
@@ -60,3 +60,12 @@ exporter.compute_unit_price_micro_lamports = 1000
# Note that this doesn't affect the rate at which transactions are published:
Copy link
Collaborator

@ali-behjati ali-behjati Jul 16, 2024

Choose a reason for hiding this comment

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

let's hide it from sample files for the time being.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

im assuming you mean the opentelemetry config?

Copy link
Collaborator

@ali-behjati ali-behjati left a comment

Choose a reason for hiding this comment

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

Nice! Can you also bump the version by minor?

@cctdaniel cctdaniel merged commit 38c45bb into main Jul 17, 2024
2 checks passed
@cctdaniel cctdaniel deleted the tracing branch July 17, 2024 08:06
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.

3 participants