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.

Sign up for GitHub

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

Jump to bottom

potential fix for std::variant implicit conversion in C++17 STL #734

Closed
wants to merge 2 commits into from
Closed

potential fix for std::variant implicit conversion in C++17 STL #734

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b38bc3f
fix implicit char * conversion to bool
lalitb May 10, 2021
b12a746
Merge branch 'main' into fix-varian-conversion
lalitb May 26, 2021
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
2 changes: 1 addition & 1 deletion exporters/ostream/test/ostream_log_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ TEST(OStreamLogExporter, LogWithStringAttributesToCerr)
auto record = exporter->MakeRecordable();

// Set resources for this log record only of type <string, string>
record->SetResource("key1", "val1");
record->SetResource("key1", std::string("val1"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we provide an explicit overload to SetResource with const char * to avoid potential future error?


// Set attributes to this log record of type <string, AttributeValue>
record->SetAttribute("a", true);
Expand Down
8 changes: 4 additions & 4 deletions exporters/ostream/test/ostream_span_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ TEST(OStreamSpanExporter, PrintSpanWithAttribute)

auto recordable = processor->MakeRecordable();

recordable->SetAttribute("attr1", "string");
recordable->SetAttribute("attr1", std::string("string"));

processor->OnEnd(std::move(recordable));

Expand Down Expand Up @@ -221,8 +221,8 @@ TEST(OStreamSpanExporter, PrintSpanWithEvents)
std::string next_str = std::to_string(next.time_since_epoch().count());

recordable->AddEvent("hello", now);
recordable->AddEvent("world", next,
common::KeyValueIterableView<Attributes>({{"attr1", "string"}}));
recordable->AddEvent(
"world", next, common::KeyValueIterableView<Attributes>({{"attr1", std::string("string")}}));

processor->OnEnd(std::move(recordable));

Expand Down Expand Up @@ -295,7 +295,7 @@ TEST(OStreamSpanExporter, PrintSpanWithLinks)

recordable->AddLink(span_context);
recordable->AddLink(span_context2,
common::KeyValueIterableView<Attributes>({{"attr1", "string"}}));
common::KeyValueIterableView<Attributes>({{"attr1", std::string("string")}}));

processor->OnEnd(std::move(recordable));

Expand Down
7 changes: 4 additions & 3 deletions sdk/src/resource/resource.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,10 @@ Resource &Resource::GetEmpty()

Resource &Resource::GetDefault()
{
static Resource default_resource({{kTelemetrySdkLanguage, "cpp"},
{kTelemetrySdkName, "opentelemetry"},
{kTelemetrySdkVersion, OPENTELEMETRY_SDK_VERSION}});
static Resource default_resource(
{{kTelemetrySdkLanguage, std::string("cpp")},
{kTelemetrySdkName, std::string("opentelemetry")},
{kTelemetrySdkVersion, std::string(OPENTELEMETRY_SDK_VERSION)}});
return default_resource;
}

Expand Down
48 changes: 25 additions & 23 deletions sdk/test/resource/resource_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,16 @@ TEST(ResourceTest, create_without_servicename)
{

opentelemetry::sdk::resource::ResourceAttributes expected_attributes = {
{"service", "backend"},
{"service", std::string("backend")},
{"version", (uint32_t)1},
{"cost", 234.23},
{"telemetry.sdk.language", "cpp"},
{"telemetry.sdk.name", "opentelemetry"},
{"telemetry.sdk.version", OPENTELEMETRY_SDK_VERSION},
{"service.name", "unknown_service"}};
{"telemetry.sdk.language", std::string("cpp")},
{"telemetry.sdk.name", std::string("opentelemetry")},
{"telemetry.sdk.version", std::string(OPENTELEMETRY_SDK_VERSION)},
{"service.name", std::string("unknown_service")}};

opentelemetry::sdk::resource::ResourceAttributes attributes = {
{"service", "backend"}, {"version", (uint32_t)1}, {"cost", 234.23}};
{"service", std::string("backend")}, {"version", (uint32_t)1}, {"cost", 234.23}};
auto resource = opentelemetry::sdk::resource::Resource::Create(attributes);
auto received_attributes = resource.GetAttributes();
for (auto &e : received_attributes)
Expand All @@ -64,13 +64,13 @@ TEST(ResourceTest, create_with_servicename)
opentelemetry::sdk::resource::ResourceAttributes expected_attributes = {
{"version", (uint32_t)1},
{"cost", 234.23},
{"telemetry.sdk.language", "cpp"},
{"telemetry.sdk.name", "opentelemetry"},
{"telemetry.sdk.version", OPENTELEMETRY_SDK_VERSION},
{"service.name", "backend"},
{"telemetry.sdk.language", std::string("cpp")},
{"telemetry.sdk.name", std::string("opentelemetry")},
{"telemetry.sdk.version", std::string(OPENTELEMETRY_SDK_VERSION)},
{"service.name", std::string("backend")},
};
opentelemetry::sdk::resource::ResourceAttributes attributes = {
{"service.name", "backend"}, {"version", (uint32_t)1}, {"cost", 234.23}};
{"service.name", std::string("backend")}, {"version", (uint32_t)1}, {"cost", 234.23}};
auto resource = opentelemetry::sdk::resource::Resource::Create(attributes);
auto received_attributes = resource.GetAttributes();
for (auto &e : received_attributes)
Expand All @@ -95,10 +95,10 @@ TEST(ResourceTest, create_with_servicename)
TEST(ResourceTest, create_with_emptyatrributes)
{
opentelemetry::sdk::resource::ResourceAttributes expected_attributes = {
{"telemetry.sdk.language", "cpp"},
{"telemetry.sdk.name", "opentelemetry"},
{"telemetry.sdk.version", OPENTELEMETRY_SDK_VERSION},
{"service.name", "unknown_service"},
{"telemetry.sdk.language", std::string("cpp")},
{"telemetry.sdk.name", std::string("opentelemetry")},
{"telemetry.sdk.version", std::string(OPENTELEMETRY_SDK_VERSION)},
{"service.name", std::string("unknown_service")},
};
opentelemetry::sdk::resource::ResourceAttributes attributes = {};
auto resource = opentelemetry::sdk::resource::Resource::Create(attributes);
Expand All @@ -115,11 +115,11 @@ TEST(ResourceTest, create_with_emptyatrributes)
TEST(ResourceTest, Merge)
{
TestResource resource1(
opentelemetry::sdk::resource::ResourceAttributes({{"service", "backend"}}));
opentelemetry::sdk::resource::ResourceAttributes({{"service", std::string("backend")}}));
TestResource resource2(
opentelemetry::sdk::resource::ResourceAttributes({{"host", "service-host"}}));
std::map<std::string, std::string> expected_attributes = {{"service", "backend"},
{"host", "service-host"}};
opentelemetry::sdk::resource::ResourceAttributes({{"host", std::string("service-host")}}));
std::map<std::string, std::string> expected_attributes = {{"service", std::string("backend")},
{"host", std::string("service-host")}};

auto merged_resource = resource1.Merge(resource2);
auto received_attributes = merged_resource.GetAttributes();
Expand All @@ -137,10 +137,12 @@ TEST(ResourceTest, Merge)

TEST(ResourceTest, MergeEmptyString)
{
TestResource resource1({{"service", "backend"}, {"host", "service-host"}});
TestResource resource2({{"service", ""}, {"host", "another-service-host"}});
std::map<std::string, std::string> expected_attributes = {{"service", ""},
{"host", "another-service-host"}};
TestResource resource1(
{{"service", std::string("backend")}, {"host", std::string("service-host")}});
TestResource resource2(
{{"service", std::string("")}, {"host", std::string("another-service-host")}});
std::map<std::string, std::string> expected_attributes = {
{"service", std::string("")}, {"host", std::string("another-service-host")}};

auto merged_resource = resource1.Merge(resource2);
auto received_attributes = merged_resource.GetAttributes();
Expand Down
4 changes: 2 additions & 2 deletions sdk/test/trace/tracer_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ TEST(Tracer, StartSpanWithAttributes)

// Start a span with all supported scalar attribute types.
tracer
->StartSpan("span 1", {{"attr1", "string"},
->StartSpan("span 1", {{"attr1", std::string("string")},
{"attr2", false},
{"attr1", 314159},
{"attr3", (unsigned int)314159},
Expand All @@ -227,7 +227,7 @@ TEST(Tracer, StartSpanWithAttributes)
{"attr6", (int64_t)-20},
{"attr7", (uint64_t)20},
{"attr8", 3.1},
{"attr9", "string"}})
{"attr9", std::string("string")}})
->End();

// Start a span with all supported array attribute types.
Expand Down