-
Notifications
You must be signed in to change notification settings - Fork 888
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
The name "IsRecordingEvents" misleading as it represent recording of any information, not just Events #44
Comments
Active may be confusing especially with being active in the context. So I would suggest maybe IsRecording? |
I'm good with |
|
Perhaps we need to clarify semantics of this flag even further. For instance, will name be recorded on span? There may be vendors who need to get the span name and serialize it to the wire. Even when span wasn't recorded. Do we really want to not record attributes? |
This feels low priority, but I would use the most concretely applicable term possible. |
I changed title to explain where the confusion can be from. |
I'm fine with |
IsRecording is used to record something even if it was sampled out. In case of tail based sampling I'd think sampled flag will be true anyway and will set recording to true automatically. |
* Start Recorder api * Fill out Span api * Reformat * Fill out Tracer class * Add cmake build * Add commenting * Fix formatting * Reformat * Fix date * Make mutex mutable * s/mutex_/mu_/ * Remove AddEvent with steady timestamp * Fix typo * Fill in IsRecordable * Use namespace macros * Add thread-compatible note * Add commenting
The semantics of
IsRecordingEvents
is not only about recording events but also setting attributes.Should it be called
IsActive
?The text was updated successfully, but these errors were encountered: