Skip to content

Commit

Permalink
Addressed reviews
Browse files Browse the repository at this point in the history
  • Loading branch information
snehilchopra committed Jul 26, 2020
1 parent 1f0bc9c commit 381d435
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 9 deletions.
5 changes: 4 additions & 1 deletion sdk/include/opentelemetry/sdk/trace/batch_span_processor.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "opentelemetry/sdk/trace/processor.h"
#include <thread>
#include <condition_variable>
#include <atomic>

OPENTELEMETRY_BEGIN_NAMESPACE
namespace sdk
Expand Down Expand Up @@ -140,7 +141,9 @@ class BatchSpanProcessor : public SpanProcessor
std::unique_ptr<common::CircularBuffer<Recordable>> buffer_;

/* Important boolean flags to handle the workflow of the processor */
bool is_shutdown_{false}, is_force_flush_{false}, is_force_flush_notified_{false};
bool is_shutdown_{false};
bool is_force_flush_{false};
bool is_force_flush_notified_{false};
};

} // trace
Expand Down
2 changes: 1 addition & 1 deletion sdk/src/trace/batch_span_processor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ void BatchSpanProcessor::ForceFlush(std::chrono::microseconds timeout) noexcept
while(is_force_flush_ == true && timeout_left.count() > 0) {
auto start = std::chrono::steady_clock::now();
cv_.notify_one();
std::this_thread::sleep_for(std::chrono::nanoseconds(50));
// std::this_thread::sleep_for(std::chrono::nanoseconds(50));
auto end = std::chrono::steady_clock::now();
timeout_left -= std::chrono::duration_cast<std::chrono::microseconds>(end-start);
}
Expand Down
14 changes: 7 additions & 7 deletions sdk/test/trace/batch_span_processor_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ class BatchSpanProcessorTestPeer : public testing::Test
for(int i = 0; i < num_spans; ++i)
{
test_spans->push_back(processor->MakeRecordable());
static_cast<sdk::trace::SpanData*>(test_spans->at(i).get())->SetName("Span " + i);
static_cast<sdk::trace::SpanData*>(test_spans->at(i).get())->SetName("Span " + std::to_string(i));
}

return test_spans;
Expand Down Expand Up @@ -150,7 +150,7 @@ TEST_F(BatchSpanProcessorTestPeer, TestShutdown)
EXPECT_EQ(num_spans, spans_received->size());
for(int i = 0; i < num_spans; ++i)
{
EXPECT_EQ("Span " + i, spans_received->at(i)->GetName());
EXPECT_EQ("Span " + std::to_string(i), spans_received->at(i)->GetName());
}

EXPECT_TRUE(*is_shutdown);
Expand All @@ -177,7 +177,7 @@ TEST_F(BatchSpanProcessorTestPeer, TestForceFlush)
EXPECT_EQ(num_spans, spans_received->size());
for(int i = 0; i < num_spans; ++i)
{
EXPECT_EQ("Span " + i, spans_received->at(i)->GetName());
EXPECT_EQ("Span " + std::to_string(i), spans_received->at(i)->GetName());
}

// Create some more spans to make sure that the processor still works
Expand All @@ -191,7 +191,7 @@ TEST_F(BatchSpanProcessorTestPeer, TestForceFlush)
EXPECT_EQ(num_spans*2, spans_received->size());
for(int i = 0; i < num_spans; ++i)
{
EXPECT_EQ("Span " + i%num_spans, spans_received->at(i)->GetName());
EXPECT_EQ("Span " + std::to_string(i%num_spans), spans_received->at(i)->GetName());
}
}

Expand Down Expand Up @@ -268,7 +268,7 @@ TEST_F(BatchSpanProcessorTestPeer, TestManySpansLossLess)
EXPECT_EQ(num_spans, spans_received->size());
for(int i = 0; i < num_spans; ++i)
{
EXPECT_EQ("Span " + i, spans_received->at(i)->GetName());
EXPECT_EQ("Span " + std::to_string(i), spans_received->at(i)->GetName());
}
}

Expand Down Expand Up @@ -304,8 +304,8 @@ TEST_F(BatchSpanProcessorTestPeer, TestScheduleDelayMillis)
EXPECT_EQ(max_export_batch_size, spans_received->size());
for(size_t i = 0; i < max_export_batch_size; ++i)
{
EXPECT_EQ("Span " + i, spans_received->at(i)->GetName());
EXPECT_EQ("Span " + std::to_string(i), spans_received->at(i)->GetName());
}
}

OPENTELEMETRY_END_NAMESPACE
OPENTELEMETRY_END_NAMESPACE

0 comments on commit 381d435

Please sign in to comment.