Added a threadsafe recordable#9
Conversation
| #include "opentelemetry/sdk/trace/processor.h" | ||
| #include "opentelemetry/sdk/trace/recordable.h" | ||
| #include "opentelemetry/sdk/trace/span_data.h" | ||
| #include "opentelemetry/ext/zpages/threadsafe_span_data.h" |
There was a problem hiding this comment.
Please sort the includes alphabetically.
There was a problem hiding this comment.
I got rid of this change, I'll add it in the next PR to switch form span data to threadsafe span data
| std::string status_desc_; | ||
| std::unordered_map<std::string, SpanDataAttributeValue> attributes_; | ||
| AttributeConverter converter_; | ||
| mutable std::mutex mutex_; |
There was a problem hiding this comment.
The mutex should be declared before the variables. Otherwise it will be destructed before the variables it "guards".
| start_time_ = start_time; | ||
| } | ||
|
|
||
| void SetDuration(std::chrono::nanoseconds duration) noexcept override { duration_ = duration; } |
jajanet
left a comment
jajanet
left a comment
There was a problem hiding this comment.
General lock stuff discussed from the standup
| * @return the trace id for this span | ||
| */ | ||
| opentelemetry::trace::TraceId GetTraceId() const noexcept { | ||
| std::unique_lock<std::mutex> lock(mutex_); |
There was a problem hiding this comment.
I would change all the getter functions to use shared_lock for spicy concurrent reads. Those are thread safe and helps speed up the span for whenever getters are called simultaneously
There was a problem hiding this comment.
If you aren't using shared_lock, I think lock_guard is better than unique_lock for most cases since it's lighter and simpler. Any reason in particular on why you prefer unique?
There was a problem hiding this comment.
shared_lock makes sense only if the mutex is shared, this allows the concurrent behavior that you are talking about. I changed them to lock_guard.
3 participants
No description provided.