Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 31 additions & 3 deletions api/include/opentelemetry/context/runtime_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -175,15 +175,30 @@ class ThreadLocalContextStorage : public RuntimeContextStorage
// Return the current context.
Context GetCurrent() noexcept override { return GetStack().Top(); }

// Resets the context to a previous value stored in the
// passed in token. Returns true if successful, false otherwise
// Resets the context to the value previous to the passed in token. This will
// also detach all child contexts of the passed in token.
// Returns true if successful, false otherwise.
bool Detach(Token &token) noexcept override
{
if (!(token == GetStack().Top()))
// In most cases, the context to be detached is on the top of the stack.
if (token == GetStack().Top())
{
GetStack().Pop();
return true;
}

if (!GetStack().Contains(token))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I guess in 99% of the chance GetStack().Top() would equal to the token, so it might worth a special case optimization (might save few CPU cycles on L188?):

if (token == GetStack().Top())
{
  GetStack().Pop();
  return true;
}

// check if token exists on the stack and pop items above it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point. Added.

{
return false;
}

while (!(token == GetStack().Top()))
{
GetStack().Pop();
}

GetStack().Pop();

return true;
}

Expand Down Expand Up @@ -214,6 +229,19 @@ class ThreadLocalContextStorage : public RuntimeContextStorage
return base_[size_];
}

bool Contains(const Token &token) const noexcept
{
for (size_t pos = size_; pos > 0; --pos)
{
if (token == base_[pos - 1])
{
return true;
}
}

return false;
}

// Returns the Context at the top of the stack.
Context Top() const noexcept
{
Expand Down
42 changes: 38 additions & 4 deletions api/test/context/runtime_context_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ TEST(RuntimeContextTest, GetCurrent)
std::map<std::string, context::ContextValue> map_test = {{"test_key", (int64_t)123}};
context::Context test_context = context::Context(map_test);
auto old_context = context::RuntimeContext::Attach(test_context);
EXPECT_TRUE(context::RuntimeContext::GetCurrent() == test_context);
EXPECT_EQ(context::RuntimeContext::GetCurrent(), test_context);
}

// Tests that detach resets the context to the previous context
Expand All @@ -25,7 +25,7 @@ TEST(RuntimeContextTest, Detach)
auto foo_context_token = context::RuntimeContext::Attach(foo_context);

foo_context_token.reset();
EXPECT_TRUE(context::RuntimeContext::GetCurrent() == test_context);
EXPECT_EQ(context::RuntimeContext::GetCurrent(), test_context);
test_context_token.reset();
}

Expand All @@ -34,9 +34,8 @@ TEST(RuntimeContextTest, DetachWrongContext)
{
std::map<std::string, context::ContextValue> map_test = {{"test_key", (int64_t)123}};
context::Context test_context = context::Context(map_test);
context::Context foo_context = context::Context(map_test);
auto test_context_token = context::RuntimeContext::Attach(test_context);
auto foo_context_token = context::RuntimeContext::Attach(foo_context);
EXPECT_TRUE(context::RuntimeContext::Detach(*test_context_token));
EXPECT_FALSE(context::RuntimeContext::Detach(*test_context_token));
}

Expand Down Expand Up @@ -96,3 +95,38 @@ TEST(RuntimeContextTest, GetValueOtherContext)
context::Context foo_context = context::Context("foo_key", (int64_t)596);
EXPECT_EQ(nostd::get<int64_t>(context::RuntimeContext::GetValue("foo_key", &foo_context)), 596);
}

// Test that any possible order of context detaching doesn't mess up the stack.
TEST(RuntimeContextTest, DetachOutOfOrder)
{
std::vector<size_t> indices;
indices.push_back(0);
indices.push_back(1);
indices.push_back(2);
indices.push_back(3);

std::vector<context::Context> contexts;
for (auto i : indices)
{
contexts.push_back(context::Context("index", i));
}

do
{
std::vector<nostd::unique_ptr<context::Token>> tokens;

for (auto &c : contexts)
{
tokens.push_back(context::RuntimeContext::Attach(c));
}

for (size_t i : indices)
{
auto token = std::move(tokens.at(i));
context::RuntimeContext::Detach(*token);
}

EXPECT_EQ(context::RuntimeContext::GetCurrent(), context::Context());

} while (std::next_permutation(indices.begin(), indices.end()));
}