Add warning about assigning the uninstaller to '_' #372
Conversation
|
|
Codecov Report
@@ Coverage Diff @@
## master #372 +/- ##
=======================================
Coverage 53.52% 53.52%
=======================================
Files 72 72
Lines 6117 6117
=======================================
Hits 3274 3274
Misses 2843 2843
Continue to review full report at Codecov.
|
README.md
Outdated
| @@ -37,6 +37,8 @@ use opentelemetry::{exporter::trace::stdout, trace::Tracer}; | |||
|
|
|||
| fn main() -> Result<(), Box<dyn std::error::Error + Send + Sync + 'static>> { | |||
| // Create a new instrumentation pipeline | |||
| // Note: using `_` instead of assigning to a variable will immediately | |||
| // drop the tracer and uninstall it so make sure to properly assign it. | |||
There was a problem hiding this comment.
May be clearer to have this say that it will drop the uninstall guard and make the tracer a no-op as the tracer won't be dropped unless you assign it to _ as well.
There was a problem hiding this comment.
Ah yes, that is maybe not crystal clear.
|
Would be nice if a must assign was available here, but a good step would be to find all of the exporter's |
It is common rust convention to use _ to signify an unused variable. Because we _are_ using this (until the end of the block) it is helpful to explain the intent.
It is common rust convention to use _ to signify an unused variable which, to new users who don't understand the inner workings,
_uninstallis.When integrating the library I spent an hour or so wondering why I was getting no logs until I realised that using
_drops the value immediately after calling and not when the_goes out of scope. Obviously this is my fault, but to prevent people from dealing with the same thing I thought a warning front-and-center would be nice.In the long run an alternative to the
must_uselint (must_assignor similar) may be a nice alternative.If anyone has some suggestions on where to put this in the code itself rather than just the readme I'm happy to include the warning in the appropriate places.