Skip to content

Commit

Permalink
[SDK] Fix GetLogger with empty library name (#2398)
Browse files Browse the repository at this point in the history
  • Loading branch information
ThomsonTan authored Nov 10, 2023
1 parent 35a9362 commit 86ee886
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 12 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ Increment the:
[#2385](https://github.com/open-telemetry/opentelemetry-cpp/pull/2385)
* [API] Add a new AddLink() operation to Span
[#2380](https://github.com/open-telemetry/opentelemetry-cpp/pull/2380)
* [SDK] Fix GetLogger with empty library
name[#2398](https://github.com/open-telemetry/opentelemetry-cpp/pull/2398)

Important changes:

Expand Down
21 changes: 9 additions & 12 deletions sdk/src/logs/logger_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ nostd::shared_ptr<opentelemetry::logs::Logger> LoggerProvider::GetLogger(
nostd::string_view schema_url,
const opentelemetry::common::KeyValueIterable &attributes) noexcept
{
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/data-model.md#field-instrumentationscope
if (library_name.empty())
{
library_name = logger_name;
}

// Ensure only one thread can read/write from the map of loggers
std::lock_guard<std::mutex> lock_guard{lock_};

Expand Down Expand Up @@ -84,18 +90,9 @@ nostd::shared_ptr<opentelemetry::logs::Logger> LoggerProvider::GetLogger(
}
*/

// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/data-model.md#field-instrumentationscope
std::unique_ptr<instrumentationscope::InstrumentationScope> lib;
if (library_name.empty())
{
lib = instrumentationscope::InstrumentationScope::Create(logger_name, library_version,
schema_url, attributes);
}
else
{
lib = instrumentationscope::InstrumentationScope::Create(library_name, library_version,
schema_url, attributes);
}
std::unique_ptr<instrumentationscope::InstrumentationScope> lib =
instrumentationscope::InstrumentationScope::Create(library_name, library_version, schema_url,
attributes);

loggers_.push_back(std::shared_ptr<opentelemetry::sdk::logs::Logger>(
new Logger(logger_name, context_, std::move(lib))));
Expand Down
14 changes: 14 additions & 0 deletions sdk/test/logs/logger_provider_sdk_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,20 @@ TEST(LoggerProviderSDK, EventLoggerProviderFactory)
auto event_logger = elp->CreateEventLogger(logger1, "otel-cpp.test");
}

TEST(LoggerPviderSDK, LoggerEquityCheck)
{
auto lp = std::shared_ptr<logs_api::LoggerProvider>(new LoggerProvider());
nostd::string_view schema_url{"https://opentelemetry.io/schemas/1.11.0"};

auto logger1 = lp->GetLogger("logger1", "opentelelemtry_library", "", schema_url);
auto logger2 = lp->GetLogger("logger1", "opentelelemtry_library", "", schema_url);
EXPECT_EQ(logger1, logger2);

auto logger3 = lp->GetLogger("logger3");
auto another_logger3 = lp->GetLogger("logger3");
EXPECT_EQ(logger3, another_logger3);
}

class DummyLogRecordable final : public opentelemetry::sdk::logs::Recordable
{
public:
Expand Down

2 comments on commit 86ee886

@github-actions
Copy link

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'OpenTelemetry-cpp sdk Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 86ee886 Previous: 35a9362 Ratio
BM_LockFreeBuffer/4 7670276.165008545 ns/iter 1082222.4324722423 ns/iter 7.09
BM_AlwaysOffSamplerConstruction 1.9450961144226064 ns/iter 0.8064599240033182 ns/iter 2.41
BM_AlwaysOnSamplerConstruction 2.0287201098149836 ns/iter 0.8155888145205716 ns/iter 2.49

This comment was automatically generated by workflow using github-action-benchmark.

@github-actions
Copy link

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'OpenTelemetry-cpp api Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 86ee886 Previous: 35a9362 Ratio
BM_ExtractBaggageHavingTenEntries 2.143775080867261 ns/iter 0.7805287642534047 ns/iter 2.75
BM_ExtractBaggageWith180Entries 1.9457988954335963 ns/iter 0.8053844692459495 ns/iter 2.42
BM_SpinLockThrashing/1/process_time/real_time 0.285157376090012 ms/iter 0.0931410677103884 ms/iter 3.06
BM_SpanIdDefaultConstructor 2.029090644017264 ns/iter 0.6252161014842883 ns/iter 3.25
BM_SpanIdConstructor 2.031324086568636 ns/iter 0.6294824959324 ns/iter 3.23
BM_SpanIdIsValid 1.86633647360865 ns/iter 0.6244716276844675 ns/iter 2.99

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.