Skip to content

Using copies of threadsafe span data instead of sample span data#15

Open
kmanghat wants to merge 21 commits into
zpages-tracez-server-class-separationfrom
data-aggregator-modifications
Open

Using copies of threadsafe span data instead of sample span data#15
kmanghat wants to merge 21 commits into
zpages-tracez-server-class-separationfrom
data-aggregator-modifications

Conversation

@kmanghat
Copy link
Copy Markdown
Owner

No description provided.


ThreadsafeSpanData (const ThreadsafeSpanData &threadsafe_span_data)
{
std::lock_guard<std::mutex> lock(threadsafe_span_data.mutex_);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

How does it access the variables directly if they are private? Can you please add a test that verifies that all variables are copied?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

I believe you can access private variables within an object even if the object is of a different instance. I think that's how default copy constructors work, I could be wrong.
I am confident that all variables are copied over since all the aggregator tests pass(which check if things like duration, span name etc. are present in the sample spans) and also I am able to view this information in the zpage server. Where would you like me to add tests that verify that all variables are copied in threadsafe span data?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Interesting, I wasn't aware of that. Thanks for the explanation!

BTW, you can change the copy constructor to be something like this:

private:
ThreadsafeSpanData(const ThreadsafeSpanData &a, const std::lock_guardstd::mutex &)
: span_name(a.span_name), span_id(a.span_id), ... {}
public:
ThreadsafeSpanData(const ThreadsafeSpanData &a) : A(a, std::lock_guardstd::mutex(a.mtx)) {}

See the explanation here: https://www.justsoftwaresolutions.co.uk/threading/thread-safe-copy-constructors.html

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.

6 participants