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

Why does octocrab need tracing as a feature? #457

Closed
suryapandian opened this issue Sep 5, 2023 · 4 comments
Closed

Why does octocrab need tracing as a feature? #457

suryapandian opened this issue Sep 5, 2023 · 4 comments

Comments

@suryapandian
Copy link
Contributor

Recently while working on this PR, we noticed the below:

Octocrab octocrab tries to not download tracing libraries for users who do not use tracing feature. However,tracing downloaded by default by hyper. Since Octocrab depends on hyper, tracing library gets downloaded by default.

Unless all the upstream libraries have similar feature flag, the feature flag tracing of octocrab does not seem to have any benefit.

Can we remove the tracing feature flag? if not, why?
If the feature flag serves other purposes, please let me know.

I am happy to work on this once you give your thoughts and direction 🙋‍♀️

@XAMPPRocky
Copy link
Owner

Thank you for your issue! You're right that it doesn't save a dependency currently. however there is a compile time and runtime cost to tracing/logging, that is removed with the feature.

Also the goal is to eventually remove the hyper dependency entirely so users can choose their HTTP client library, so it will be more relevant then.

@suryapandian
Copy link
Contributor Author

however there is a compile time and runtime cost to tracing/logging, that is removed with the feature.
wow! wasnt aware that simple tracing log statements could affect compile time and runtime.

Thanks for the clarification, closing this issue in that case. I have also raised an issue upstream hyperium/hyper#3304 requesting them to feature flag tracing, i will probably try implementing that in hyper, not sure if it would be good-first-issue for me to pick up tho. 🤔

@XAMPPRocky
Copy link
Owner

To be clear, depending on your application, the cost is incredibly minimal and it will usually never be a bottleneck, but there's always some cost, and it can also add extra noise to logs you might not want.

@suryapandian
Copy link
Contributor Author

Thanks for clarification

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

No branches or pull requests

2 participants