[ETW Exporter] - ETW provider handle cleanup by lalitb · Pull Request #1322 · open-telemetry/opentelemetry-cpp

lalitb · 2022-04-08T08:25:46Z

Fixes:

ETW provider handle is not cleaned-up properly when ETW logger is destroyed. While the handle is deregistered, it is not removed properly from the list of active providers ( because deletion is done wrongly on the copy of the provider map). The provider map gets corrupted and results in event loss if the logger with the same name is created again.

Changes

Tiny one character change to ensure deletion of provider handle happens through the reference of the provider list, and not from the copy.

For significant contributions please make sure you have completed the following items:

CHANGELOG.md updated for non-trivial changes
Unit tests have been added
Changes in public API reviewed

codecov · 2022-04-08T08:35:22Z

Codecov Report

Merging #1322 (17a3b7a) into main (3122254) will increase coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1322      +/-   ##
==========================================
+ Coverage   91.35%   91.36%   +0.02%     
==========================================
  Files         217      217              
  Lines        7842     7842              
==========================================
+ Hits         7163     7164       +1     
+ Misses        679      678       -1

Impacted Files	Coverage Δ
...include/opentelemetry/sdk/common/circular_buffer.h	`100.00% <0.00%> (+2.00%)`	⬆️

* install sdk config (#1273) * Bump actions/cache from 2 to 3 (#1277) * Add owent as an Approver (#1276) * add owent as reviewer * fix order * Disable benchmark action failure (#1284) * metrics exemplar round 1 (#1264) * [Metrics SDK] - fix spelling (AggregationTemporarily to AggregationTemporality) (#1288) * fix compilation error with protobuf 3.5 (#1289) * Fix span SetAttribute crash (#1283) * Synchronous Metric collection (Delta , Cumulative) (#1265) * Rename `http_client_curl` to `opentelemetry_http_client_curl` (#1301) Signed-off-by: owent <admin@owent.net> * Don't show coverage annotation for pull requests (#1304) * Implement periodic exporting metric reader (#1286) * Add `async-changes` branch to pull_request of github action (#1309) Signed-off-by: owent <admin@owent.net> * Add InstrumentationInfo and Resource to the metrics data to be exported. (#1299) * Excempt should be applied on issue instead of PR (#1316) * Bump codecov/codecov-action from 2.1.0 to 3 (#1318) * Move public definitions into `opentelemetry_api`. (#1314) Signed-off-by: owent <admin@owent.net> * Add building test without RTTI (#1294) * Remove implicitly deleted default constructor (#1267) Co-authored-by: Tom Tan <Tom.Tan@microsoft.com> Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> * [ETW Exporter] - ETW provider handle cleanup (#1322) * Bump actions/stale from 4 to 5 (#1323) * ostream metrics example (#1312) * Prepare v1.3.0 release (#1324) * Update yield logic for ARM processor (#1325) * Fix for #1292 (#1326) * Implement Merge and Diff operation for Histogram Aggregation (#1303) * fix metrics compiler warnings (#1328) * Replace deprecated googletest API (#1327) * Remove redundant tail / in CMake install (#1329) * dependencies image as artifact (#1333) Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> * metrics histogram example (#1330) * Link `opentelemetry_ext` with `opentelemetry_api` (#1336) * ostream metrics cmake (#1344) * Fix conflicts Signed-off-by: owent <admin@owent.net> * Using clang-format-10 to format codes(clang-format-14 has a different output) Signed-off-by: owent <admin@owent.net> Co-authored-by: Ehsan Saei <71217171+esigo@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> Co-authored-by: Tom Tan <Tom.Tan@microsoft.com> Co-authored-by: Ben Landrum <71329856+benlandrum@users.noreply.github.com> Co-authored-by: jmanjon <67091862+juandemanjon@users.noreply.github.com>

* install sdk config (#1273) * Bump actions/cache from 2 to 3 (#1277) * Add owent as an Approver (#1276) * add owent as reviewer * fix order * Disable benchmark action failure (#1284) * metrics exemplar round 1 (#1264) * [Metrics SDK] - fix spelling (AggregationTemporarily to AggregationTemporality) (#1288) * fix compilation error with protobuf 3.5 (#1289) * Fix span SetAttribute crash (#1283) * Synchronous Metric collection (Delta , Cumulative) (#1265) * Rename `http_client_curl` to `opentelemetry_http_client_curl` (#1301) Signed-off-by: owent <admin@owent.net> * Don't show coverage annotation for pull requests (#1304) * Implement periodic exporting metric reader (#1286) * Add `async-changes` branch to pull_request of github action (#1309) Signed-off-by: owent <admin@owent.net> * Add InstrumentationInfo and Resource to the metrics data to be exported. (#1299) * Excempt should be applied on issue instead of PR (#1316) * Bump codecov/codecov-action from 2.1.0 to 3 (#1318) * Move public definitions into `opentelemetry_api`. (#1314) Signed-off-by: owent <admin@owent.net> * Add building test without RTTI (#1294) * Remove implicitly deleted default constructor (#1267) Co-authored-by: Tom Tan <Tom.Tan@microsoft.com> Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> * [ETW Exporter] - ETW provider handle cleanup (#1322) * Bump actions/stale from 4 to 5 (#1323) * ostream metrics example (#1312) * Prepare v1.3.0 release (#1324) * Update yield logic for ARM processor (#1325) * Fix for #1292 (#1326) * Implement Merge and Diff operation for Histogram Aggregation (#1303) * fix metrics compiler warnings (#1328) * Replace deprecated googletest API (#1327) * Remove redundant tail / in CMake install (#1329) * dependencies image as artifact (#1333) Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> * metrics histogram example (#1330) * Link `opentelemetry_ext` with `opentelemetry_api` (#1336) * ostream metrics cmake (#1344) * prometheus exporter (#1331) * remove exporter registration to meter provider (#1350) * Bump github/codeql-action from 1 to 2 (#1351) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Add explicit type cast in baggage UrlDecode (#1353) * Fix scalar delete against array (#1356) * conditional include for codecvt header (#1355) * Add missing include guard (#1357) * Use latest TraceLoggingDynamic.h (#1354) * prometheus example (#1332) * Fix output time in metrics OStream exporter (#1346) * Simplify SDK Configuration: Use View with default aggregation if no matching View is configured (#1358) * Fix class member initialization order (#1360) * codecov ignore (#1364) * export opentelemetry_otlp_recordable (#1365) * Disable test on prometheus-cpp which not need (#1363) * Enable metric collection for Async Instruments - Delta and Cumulative (#1334) * fix baggage propagation for empty/invalid baggage context (#1367) * Fix empty tracestate header propagation (#1373) * Bump docker/setup-qemu-action from 1 to 2 (#1375) * Add noexcept/const qualifier at missing places for Trace API. (#1374) * fix noxcept * fix etw * Bump docker/build-push-action from 2 to 3 (#1377) * Bump docker/setup-buildx-action from 1 to 2 (#1376) * [Metrics SDK] Remove un-necessary files. (#1379) * reuse temporal metric storage for sync storage (#1369) * Prometheus exporter meters and instrument name (#1378) * Fix sharing resource in batched exported spans (#1386) Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> * fix: missing link to nlohmann_json (#1390) * Getting started document using ostream exporter (#1394) Co-authored-by: Reiley Yang <reyang@microsoft.com> * Connect async storage with async instruments (#1388) * get span_id from context when Logger::Log received invalid span_id (#1398) * Alpine image (#1382) * Upgrade proto to v0.17.0, update log data model (#1383) * Prepare v1.4.0 release (#1404) * Fix vcpkg package name in doc (#1392) * Document Getting Started with Prometheus and Grafana (#1396) Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> * fix OTEL_INTERNAL_LOG_INFO (#1407) * fix: WaitOnSocket select error when sockfd above FD_SETSIZE (#1410) * [BUILD] fix nlohmann_json's (third party) include dir (#1415) * [Metrics API/SDK] - Pass state to async callback function. (#1408) * Copy string_view passed to ETW exporter in PropertyVariant (#1425) * Fix ETW log exporter header inclusion (#1426) * [Metrics SDK] Only record non-negative / finite / Non-NAN histogram values (#1427) * validate histogram value * handle Nan * add changelog * divide by 0 error on windows * fix markdown lint * Fix global log handle symbols when using dlopen (#1420) * Add attributes/dimensions to metrics ostream exporter (#1400) * Log current timestamp instead of epoch time (#1434) * install sdk-config.h (#1419) * Fix GettingStarted documentation for Jaeger HTTP exporter (#1347) (#1439) * fix histogram (#1440) * Upgrade nlohmann_json to 3.10.5 (#1438) (#1441) * Fixed broken link to OpenTelemetry.io (#1445) (#1446) Co-authored-by: Ehsan Saei <71217171+esigo@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> Co-authored-by: Tom Tan <Tom.Tan@microsoft.com> Co-authored-by: Ben Landrum <71329856+benlandrum@users.noreply.github.com> Co-authored-by: jmanjon <67091862+juandemanjon@users.noreply.github.com> Co-authored-by: Zsolt Bölöny <bolony.zsolt@gmail.com> Co-authored-by: Leo Di Donato <leodidonato@gmail.com> Co-authored-by: Reiley Yang <reyang@microsoft.com> Co-authored-by: Lalit Kumar Bhasin <labhas@microsoft.com> Co-authored-by: Hamed Mansouri <hamed0381@gmail.com> Co-authored-by: ztao <t@taozj.org> Co-authored-by: Flier Lu <flier@users.noreply.github.com> Co-authored-by: Marc Alff <marc.alff@free.fr> Co-authored-by: Marc Alff <marc.alff@oracle.com>

* install sdk config (#1273) * Bump actions/cache from 2 to 3 (#1277) * Add owent as an Approver (#1276) * add owent as reviewer * fix order * Disable benchmark action failure (#1284) * metrics exemplar round 1 (#1264) * [Metrics SDK] - fix spelling (AggregationTemporarily to AggregationTemporality) (#1288) * fix compilation error with protobuf 3.5 (#1289) * Fix span SetAttribute crash (#1283) * Synchronous Metric collection (Delta , Cumulative) (#1265) * Rename `http_client_curl` to `opentelemetry_http_client_curl` (#1301) Signed-off-by: owent <admin@owent.net> * Don't show coverage annotation for pull requests (#1304) * Implement periodic exporting metric reader (#1286) * Add `async-changes` branch to pull_request of github action (#1309) Signed-off-by: owent <admin@owent.net> * Add InstrumentationInfo and Resource to the metrics data to be exported. (#1299) * Excempt should be applied on issue instead of PR (#1316) * Bump codecov/codecov-action from 2.1.0 to 3 (#1318) * Move public definitions into `opentelemetry_api`. (#1314) Signed-off-by: owent <admin@owent.net> * Add building test without RTTI (#1294) * Remove implicitly deleted default constructor (#1267) Co-authored-by: Tom Tan <Tom.Tan@microsoft.com> Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> * [ETW Exporter] - ETW provider handle cleanup (#1322) * Bump actions/stale from 4 to 5 (#1323) * ostream metrics example (#1312) * Prepare v1.3.0 release (#1324) * Update yield logic for ARM processor (#1325) * Fix for #1292 (#1326) * Implement Merge and Diff operation for Histogram Aggregation (#1303) * fix metrics compiler warnings (#1328) * Replace deprecated googletest API (#1327) * Remove redundant tail / in CMake install (#1329) * dependencies image as artifact (#1333) Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> * metrics histogram example (#1330) * Link `opentelemetry_ext` with `opentelemetry_api` (#1336) * ostream metrics cmake (#1344) * prometheus exporter (#1331) * remove exporter registration to meter provider (#1350) * Bump github/codeql-action from 1 to 2 (#1351) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Add explicit type cast in baggage UrlDecode (#1353) * Fix scalar delete against array (#1356) * conditional include for codecvt header (#1355) * Add missing include guard (#1357) * Use latest TraceLoggingDynamic.h (#1354) * prometheus example (#1332) * Fix output time in metrics OStream exporter (#1346) * Simplify SDK Configuration: Use View with default aggregation if no matching View is configured (#1358) * Fix class member initialization order (#1360) * codecov ignore (#1364) * export opentelemetry_otlp_recordable (#1365) * Disable test on prometheus-cpp which not need (#1363) * Enable metric collection for Async Instruments - Delta and Cumulative (#1334) * fix baggage propagation for empty/invalid baggage context (#1367) * Fix empty tracestate header propagation (#1373) * Bump docker/setup-qemu-action from 1 to 2 (#1375) * Add noexcept/const qualifier at missing places for Trace API. (#1374) * fix noxcept * fix etw * Bump docker/build-push-action from 2 to 3 (#1377) * Bump docker/setup-buildx-action from 1 to 2 (#1376) * [Metrics SDK] Remove un-necessary files. (#1379) * reuse temporal metric storage for sync storage (#1369) * Prometheus exporter meters and instrument name (#1378) * Fix sharing resource in batched exported spans (#1386) Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> * fix: missing link to nlohmann_json (#1390) * Getting started document using ostream exporter (#1394) Co-authored-by: Reiley Yang <reyang@microsoft.com> * Connect async storage with async instruments (#1388) * get span_id from context when Logger::Log received invalid span_id (#1398) * Alpine image (#1382) * Upgrade proto to v0.17.0, update log data model (#1383) * Prepare v1.4.0 release (#1404) * Fix vcpkg package name in doc (#1392) * Document Getting Started with Prometheus and Grafana (#1396) Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> * fix OTEL_INTERNAL_LOG_INFO (#1407) * fix: WaitOnSocket select error when sockfd above FD_SETSIZE (#1410) * [BUILD] fix nlohmann_json's (third party) include dir (#1415) * [Metrics API/SDK] - Pass state to async callback function. (#1408) * Copy string_view passed to ETW exporter in PropertyVariant (#1425) * Fix ETW log exporter header inclusion (#1426) * [Metrics SDK] Only record non-negative / finite / Non-NAN histogram values (#1427) * validate histogram value * handle Nan * add changelog * divide by 0 error on windows * fix markdown lint * Fix global log handle symbols when using dlopen (#1420) * Add attributes/dimensions to metrics ostream exporter (#1400) * Log current timestamp instead of epoch time (#1434) * install sdk-config.h (#1419) * Fix GettingStarted documentation for Jaeger HTTP exporter (#1347) (#1439) * fix histogram (#1440) * Upgrade nlohmann_json to 3.10.5 (#1438) (#1441) * Fixed broken link to OpenTelemetry.io (#1445) (#1446) * Fix variables inizialization (#1430) * Remove `AsyncBatch*Processor`, implement [opentelemetry-specification#2452](open-telemetry/opentelemetry-specification#2452) Signed-off-by: WenTao Ou <admin@owent.net> * Remove invalid changelog Signed-off-by: WenTao Ou <admin@owent.net> * Fix compiling problems without `ENABLE_ASYNC_EXPORT` Signed-off-by: WenTao Ou <admin@owent.net> * Always return `ExportResult::kSuccess` Signed-off-by: WenTao Ou <admin@owent.net> Co-authored-by: Ehsan Saei <71217171+esigo@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> Co-authored-by: Tom Tan <Tom.Tan@microsoft.com> Co-authored-by: Ben Landrum <71329856+benlandrum@users.noreply.github.com> Co-authored-by: jmanjon <67091862+juandemanjon@users.noreply.github.com> Co-authored-by: Zsolt Bölöny <bolony.zsolt@gmail.com> Co-authored-by: Leo Di Donato <leodidonato@gmail.com> Co-authored-by: Reiley Yang <reyang@microsoft.com> Co-authored-by: Lalit Kumar Bhasin <labhas@microsoft.com> Co-authored-by: Hamed Mansouri <hamed0381@gmail.com> Co-authored-by: ztao <t@taozj.org> Co-authored-by: Flier Lu <flier@users.noreply.github.com> Co-authored-by: Marc Alff <marc.alff@free.fr> Co-authored-by: Marc Alff <marc.alff@oracle.com> Co-authored-by: univisionsrl <31103417+univisionsrl@users.noreply.github.com>

fix provider cleanup

dd757f6

lalitb requested a review from a team April 8, 2022 08:25

ThomsonTan reviewed Apr 8, 2022

View reviewed changes

Comment thread exporters/etw/include/opentelemetry/exporters/etw/etw_provider.h

ThomsonTan approved these changes Apr 8, 2022

View reviewed changes

lalitb added 4 commits April 8, 2022 07:39

fix

2f102f6

add comment

0f87019

remove from provider list only if removal is successful

c4ffe77

fix test case

17a3b7a

lalitb merged commit d72dcb6 into open-telemetry:main Apr 11, 2022

Navigation Menu

Uh oh!

[ETW Exporter] - ETW provider handle cleanup - #1322

[ETW Exporter] - ETW provider handle cleanup#1322
lalitb merged 5 commits into
open-telemetry:mainfrom
lalitb:etw-provider-cleanup

lalitb commented Apr 8, 2022 •

edited

Loading

Uh oh!

codecov Bot commented Apr 8, 2022 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

lalitb commented Apr 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

codecov Bot commented Apr 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lalitb commented Apr 8, 2022 •

edited

Loading

codecov Bot commented Apr 8, 2022 •

edited

Loading