From dd757f6770bef7cb0e4630754a76df4b2162d7ae Mon Sep 17 00:00:00 2001 From: Lalit Date: Fri, 8 Apr 2022 01:10:50 -0700 Subject: [PATCH 1/5] fix provider cleanup --- .../etw/include/opentelemetry/exporters/etw/etw_provider.h | 2 +- exporters/etw/test/etw_provider_test.cc | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/exporters/etw/include/opentelemetry/exporters/etw/etw_provider.h b/exporters/etw/include/opentelemetry/exporters/etw/etw_provider.h index 5b00f05574..22bc9ed0e3 100644 --- a/exporters/etw/include/opentelemetry/exporters/etw/etw_provider.h +++ b/exporters/etw/include/opentelemetry/exporters/etw/etw_provider.h @@ -203,7 +203,7 @@ class ETWProvider { std::lock_guard lock(m_providerMapLock); - auto m = providers(); + auto &m = providers(); auto it = m.begin(); while (it != m.end()) { diff --git a/exporters/etw/test/etw_provider_test.cc b/exporters/etw/test/etw_provider_test.cc index 433630270f..8c28bba97b 100644 --- a/exporters/etw/test/etw_provider_test.cc +++ b/exporters/etw/test/etw_provider_test.cc @@ -57,6 +57,7 @@ TEST(ETWProvider, CheckCloseSuccess) auto result = etw.close(handle); ASSERT_NE(result, etw.STATUS_ERROR); + ASSERT_FALSE(etw.is_registered(providerName);); } #endif From 2f102f6379e5bebb17f5537248374a4e5314ccf2 Mon Sep 17 00:00:00 2001 From: Lalit Date: Fri, 8 Apr 2022 07:39:20 -0700 Subject: [PATCH 2/5] fix --- exporters/etw/test/etw_provider_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporters/etw/test/etw_provider_test.cc b/exporters/etw/test/etw_provider_test.cc index 8c28bba97b..d3f3ba354e 100644 --- a/exporters/etw/test/etw_provider_test.cc +++ b/exporters/etw/test/etw_provider_test.cc @@ -57,7 +57,7 @@ TEST(ETWProvider, CheckCloseSuccess) auto result = etw.close(handle); ASSERT_NE(result, etw.STATUS_ERROR); - ASSERT_FALSE(etw.is_registered(providerName);); + ASSERT_FALSE(etw.is_registered(providerName)); } #endif From 0f870197d33b2c248886ff6874c7effa52ca62a6 Mon Sep 17 00:00:00 2001 From: Lalit Date: Fri, 8 Apr 2022 07:44:01 -0700 Subject: [PATCH 3/5] add comment --- exporters/etw/include/opentelemetry/exporters/etw/etw_provider.h | 1 + 1 file changed, 1 insertion(+) diff --git a/exporters/etw/include/opentelemetry/exporters/etw/etw_provider.h b/exporters/etw/include/opentelemetry/exporters/etw/etw_provider.h index 22bc9ed0e3..c300a9405e 100644 --- a/exporters/etw/include/opentelemetry/exporters/etw/etw_provider.h +++ b/exporters/etw/include/opentelemetry/exporters/etw/etw_provider.h @@ -203,6 +203,7 @@ class ETWProvider { std::lock_guard lock(m_providerMapLock); + // use reference to provider list, NOT it' copy. auto &m = providers(); auto it = m.begin(); while (it != m.end()) From c4ffe7755bf59413e6a42cca4f4050abcf21067b Mon Sep 17 00:00:00 2001 From: Lalit Date: Fri, 8 Apr 2022 11:05:45 -0700 Subject: [PATCH 4/5] remove from provider list only if removal is successful --- .../etw/include/opentelemetry/exporters/etw/etw_provider.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/exporters/etw/include/opentelemetry/exporters/etw/etw_provider.h b/exporters/etw/include/opentelemetry/exporters/etw/etw_provider.h index c300a9405e..51cca03a60 100644 --- a/exporters/etw/include/opentelemetry/exporters/etw/etw_provider.h +++ b/exporters/etw/include/opentelemetry/exporters/etw/etw_provider.h @@ -229,7 +229,10 @@ class ETWProvider } it->second.providerHandle = INVALID_HANDLE; - m.erase(it); + if (result == STATUS_OK) + { + m.erase(it); + } } return result; } From 17a3b7a2d4a53104a626be295590387c0db68274 Mon Sep 17 00:00:00 2001 From: Lalit Date: Sun, 10 Apr 2022 23:42:54 -0700 Subject: [PATCH 5/5] fix test case --- exporters/etw/test/etw_provider_test.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/exporters/etw/test/etw_provider_test.cc b/exporters/etw/test/etw_provider_test.cc index d3f3ba354e..d5ebbcad43 100644 --- a/exporters/etw/test/etw_provider_test.cc +++ b/exporters/etw/test/etw_provider_test.cc @@ -18,6 +18,7 @@ TEST(ETWProvider, ProviderIsRegisteredSuccessfully) bool registered = etw.is_registered(providerName); ASSERT_TRUE(registered); + etw.close(handle); } TEST(ETWProvider, ProviderIsNotRegisteredSuccessfully) @@ -46,6 +47,7 @@ TEST(ETWProvider, CheckOpenGUIDDataSuccessfully) auto guidStrName = uuid_name.to_string(); ASSERT_STREQ(guidStrHandle.c_str(), guidStrName.c_str()); + etw.close(handle); } TEST(ETWProvider, CheckCloseSuccess) @@ -53,8 +55,7 @@ TEST(ETWProvider, CheckCloseSuccess) std::string providerName = "OpenTelemetry-ETW-Provider"; static ETWProvider etw; - auto handle = etw.open(providerName.c_str()); - + auto handle = etw.open(providerName.c_str(), ETWProvider::EventFormat::ETW_MANIFEST); auto result = etw.close(handle); ASSERT_NE(result, etw.STATUS_ERROR); ASSERT_FALSE(etw.is_registered(providerName));