Skip to content
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

Added Logging SDK Context Injection and Logger Limit #435

Closed
wants to merge 2 commits into from
Closed
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
7 changes: 5 additions & 2 deletions sdk/include/opentelemetry/sdk/logs/logger_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
#include "opentelemetry/sdk/logs/processor.h"

// Define the maximum number of loggers that are allowed to be registered to the loggerprovider.
// TODO: Add link to logging spec once this is added to it
#define MAX_LOGGER_COUNT 100
// References spec issue https://github.com/open-telemetry/opentelemetry-specification/issues/1259
#define OTEL_MAX_LOGGER_COUNT 1000

OPENTELEMETRY_BEGIN_NAMESPACE
namespace sdk
Expand Down Expand Up @@ -95,6 +95,9 @@ class LoggerProvider final : public opentelemetry::logs::LoggerProvider,

// A mutex that ensures only one thread is using the map of loggers
std::mutex mu_;

// A noop logger that is returned by GetLogger() when OTEL_MAX_LOGGER_COUNT reached
opentelemetry::nostd::shared_ptr<opentelemetry::logs::Logger> noop_logger_;
};
} // namespace logs
} // namespace sdk
Expand Down
40 changes: 35 additions & 5 deletions sdk/src/logs/logger.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/

#include "opentelemetry/sdk/logs/logger.h"
#include "opentelemetry/sdk/trace/span_data.h"
#include "opentelemetry/trace/provider.h"

OPENTELEMETRY_BEGIN_NAMESPACE
namespace sdk
Expand Down Expand Up @@ -44,12 +46,40 @@ void Logger::log(const opentelemetry::logs::LogRecord &record) noexcept
auto record_pointer =
std::unique_ptr<opentelemetry::logs::LogRecord>(new opentelemetry::logs::LogRecord(record));

// TODO: Do not want to overwrite user-set timestamp if there already is one -
// add a flag in the API to check if timestamp is set by user already before setting timestamp
// Inject values into record if not user specified
// Timestamp
if (record_pointer->timestamp == opentelemetry::core::SystemTimestamp(std::chrono::seconds(0)))
{
record_pointer->timestamp = core::SystemTimestamp(std::chrono::system_clock::now());
}

// Severity
if (record_pointer->severity == opentelemetry::logs::Severity::kInvalid)
{
record_pointer->severity = opentelemetry::logs::Severity::kInfo;
}

auto provider = opentelemetry::trace::Provider::GetTracerProvider();
auto tracer = provider->GetTracer("foo_library");
auto span_context = tracer->GetCurrentSpan()->GetContext();

// Traceid
if (!record_pointer->trace_id.IsValid())
{
record_pointer->trace_id = span_context.trace_id();
}

// Inject timestamp if none is set
record_pointer->timestamp = core::SystemTimestamp(std::chrono::system_clock::now());
// TODO: inject traceid/spanid later
// Spanid
if (!record_pointer->span_id.IsValid())
{
record_pointer->span_id = span_context.span_id();
}

// Traceflag
if (!record_pointer->trace_flags.IsSampled())
{
record_pointer->trace_flags = span_context.trace_flags();
}

// Send the log record to the processor
processor->OnReceive(std::move(record_pointer));
Expand Down
19 changes: 8 additions & 11 deletions sdk/src/logs/logger_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ namespace sdk
namespace logs
{

LoggerProvider::LoggerProvider() noexcept : processor_{nullptr} {}
LoggerProvider::LoggerProvider() noexcept
: processor_{nullptr},
noop_logger_{
nostd::shared_ptr<opentelemetry::logs::NoopLogger>(new opentelemetry::logs::NoopLogger)}
{}

opentelemetry::nostd::shared_ptr<opentelemetry::logs::Logger> LoggerProvider::GetLogger(
opentelemetry::nostd::string_view name,
Expand All @@ -39,20 +43,13 @@ opentelemetry::nostd::shared_ptr<opentelemetry::logs::Logger> LoggerProvider::Ge
}

// Check if creating a new logger would exceed the max number of loggers
// TODO: Remove the noexcept from the API's and SDK's GetLogger(~)
/*
if (loggers_.size() > MAX_LOGGER_COUNT)
if (loggers_.size() >= OTEL_MAX_LOGGER_COUNT)
{
#if __EXCEPTIONS
throw std::length_error("Number of loggers exceeds max count");
#else
std::terminate();
#endif
// Return a noop logger
return noop_logger_;
}
*/

// If no logger with that name exists yet, create it and add it to the map of loggers

opentelemetry::nostd::shared_ptr<opentelemetry::logs::Logger> logger(
new Logger(this->shared_from_this()));
loggers_[name.data()] = logger;
Expand Down
2 changes: 2 additions & 0 deletions sdk/test/logs/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ cc_test(
"logger_sdk_test.cc",
],
deps = [
"//exporters/memory:in_memory_span_exporter",
"//sdk/src/logs",
"//sdk/src/trace",
"@com_google_googletest//:gtest_main",
],
)
Expand Down
10 changes: 4 additions & 6 deletions sdk/test/logs/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
foreach(testname logger_provider_sdk_test logger_sdk_test
simple_log_processor_test)
add_executable(${testname} "${testname}.cc")
target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES}
${CMAKE_THREAD_LIBS_INIT} opentelemetry_logs)
gtest_add_tests(
TARGET ${testname}
TEST_PREFIX logs.
TEST_LIST ${testname})
target_link_libraries(
${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT}
opentelemetry_logs opentelemetry_trace opentelemetry_exporter_in_memory)
gtest_add_tests(TARGET ${testname} TEST_PREFIX logs. TEST_LIST ${testname})
endforeach()
16 changes: 16 additions & 0 deletions sdk/test/logs/logger_provider_sdk_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,19 @@ TEST(LoggerProviderSDK, GetAndSetProcessor)
lp.SetProcessor(proc2);
ASSERT_EQ(proc2, lp.GetProcessor());
}

TEST(LoggerProviderSDK, LoggerLimit)
{
auto lp = std::shared_ptr<opentelemetry::logs::LoggerProvider>(new LoggerProvider());

// Create the maximum number of loggers
for (int i = 0; i < OTEL_MAX_LOGGER_COUNT; i++)
{
lp->GetLogger(std::to_string(i));
}

// Create two more loggers and check that they are both the same noop logger
auto logger1 = lp->GetLogger("Logger1");
auto logger2 = lp->GetLogger("Logger2");
ASSERT_EQ(logger1, logger2);
}
63 changes: 62 additions & 1 deletion sdk/test/logs/logger_sdk_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,16 @@
* limitations under the License.
*/

#include "opentelemetry/exporters/memory/in_memory_span_exporter.h"
#include "opentelemetry/sdk/logs/logger.h"
#include "opentelemetry/sdk/trace/simple_processor.h"
#include "opentelemetry/sdk/trace/tracer_provider.h"
#include "opentelemetry/trace/provider.h"

#include <gtest/gtest.h>

using namespace opentelemetry::sdk::logs;
namespace sdktrace = opentelemetry::sdk::trace;

TEST(LoggerSDK, LogToNullProcessor)
{
Expand All @@ -35,9 +40,16 @@ TEST(LoggerSDK, LogToNullProcessor)
logger->log(r);
}

// Define a global log record that will be modified when the Log() method is called
static opentelemetry::nostd::shared_ptr<opentelemetry::logs::LogRecord> record_;

class DummyProcessor : public LogProcessor
{
void OnReceive(std::unique_ptr<opentelemetry::logs::LogRecord> &&record) noexcept {}
void OnReceive(std::unique_ptr<opentelemetry::logs::LogRecord> &&record) noexcept
{
record_ = opentelemetry::nostd::shared_ptr<opentelemetry::logs::LogRecord>(
new opentelemetry::logs::LogRecord(*record.get()));
}
bool ForceFlush(std::chrono::microseconds timeout = std::chrono::microseconds(0)) noexcept
{
return true;
Expand Down Expand Up @@ -75,3 +87,52 @@ TEST(LoggerSDK, LogToAProcessor)
r.name = "Test log";
logger->log(r);
}

TEST(LoggerSDK, DefaultValueInjection)
{
// Use the DummyProcessor defined above, which saves the lastest record in the
// _record global variable
std::shared_ptr<LogProcessor> processor = std::shared_ptr<LogProcessor>(new DummyProcessor());
auto lp = std::shared_ptr<LoggerProvider>(new LoggerProvider());
lp->SetProcessor(processor);
auto logger = lp->GetLogger("Logger1");

// Log a sample log record to the processor
opentelemetry::logs::LogRecord r;
r.name = "Test log";
logger->log(r);

// Check that the log record has injected values
// Timestamp shouldn't equal 0
ASSERT_NE(record_->timestamp, opentelemetry::core::SystemTimestamp(std::chrono::seconds(0)));

// Check that the traceid, spanid, and traceflags are not valid since there is no trace context
ASSERT_FALSE(record_->trace_id.IsValid());
ASSERT_FALSE(record_->span_id.IsValid());
ASSERT_FALSE(record_->trace_flags.IsSampled());

// To test traceid/spanid/traceflags injection, initialize the tracing pipeline
std::unique_ptr<opentelemetry::exporter::memory::InMemorySpanExporter> trace_exporter(
new opentelemetry::exporter::memory::InMemorySpanExporter());
auto trace_processor = std::make_shared<sdktrace::SimpleSpanProcessor>(std::move(trace_exporter));
auto trace_provider = opentelemetry::nostd::shared_ptr<opentelemetry::trace::TracerProvider>(
new sdktrace::TracerProvider(trace_processor));
opentelemetry::trace::Provider::SetTracerProvider(trace_provider);

// Create a tracer and start a span for span context
auto tracer = trace_provider->GetTracer("foo_library");
auto span_first = tracer->StartSpan("span 1");
auto scope_first = tracer->WithActiveSpan(span_first);
auto span_second = tracer->StartSpan("span 2");

// Log a sample log record to the processor
opentelemetry::logs::LogRecord r2;
r2.name = "Test log";
logger->log(r2);

// Check that the traceid, spanid, and traceflags were injected from the span context properly
auto span_context = tracer->GetCurrentSpan()->GetContext();
ASSERT_EQ(record_->trace_id, span_context.trace_id());
ASSERT_EQ(record_->span_id, span_context.span_id());
ASSERT_EQ(record_->trace_flags, span_context.trace_flags());
}