Deprecate event logger #3285
Deprecate event logger #3285
Conversation
✅ Deploy Preview for opentelemetry-cpp-api-docs canceled.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3285 +/- ##
=======================================
Coverage 89.41% 89.41%
=======================================
Files 207 207
Lines 6457 6457
=======================================
Hits 5773 5773
Misses 684 684
|
owent
force-pushed
the
deprecate_event_logger
branch
from
37f6452 to
7c97425
Compare
| "gsl_byte": "cpp" | ||
| } //, | ||
| //"editor.formatOnSave": false | ||
| } |
There was a problem hiding this comment.
This file contains path locals to the dev machine, so it should probably not be added, at least as is.
@owent Please confirm if this is intended, or remove.
| @@ -26,8 +26,7 @@ namespace exception | |||
| * It's no longer recommended to record exceptions that are handled and do not escape the scope of a | |||
| * span. | |||
| */ | |||
| OPENTELEMETRY_DEPRECATED | |||
| static constexpr const char *kExceptionEscaped = "exception.escaped"; | |||
| OPENTELEMETRY_DEPRECATED static constexpr const char *kExceptionEscaped = "exception.escaped"; | |||
There was a problem hiding this comment.
Are these semconv modified by clang-format? Do you think we can scope it in separate PR if really required?
| - OPENTELEMETRY_LOCAL_SYMBOL | ||
| - OPENTELEMETRY_EXPORT | ||
| - OPENTELEMETRY_SANITIZER_NO_MEMORY | ||
| - OPENTELEMETRY_SANITIZER_NO_THREAD |
There was a problem hiding this comment.
Can we move this formatting in separate PR. I don't think it is related to deprecating event logger.
|
@owent thanks for the PR. The event logger related changes look good in general. However, can we move the clang formatting related changes out of scope of this PR? we can have separate PR for that if needed. |
There was a problem hiding this comment.
This change contains two independent parts:
- deprecation (abi v1) and removal (abi v2) of the event logger
- clang-format changes with compiler attributes macros
Approving this PR because both look ok.
See comments on vscode file.
@owent For future work, please separate independent changes in different PRs.
This helps with the git history, and helps when a backport is needed to an older branch for some fixes.
|
Waiting on clarifications and/or cleanup for vscode file, before merging. |
Fixes #3193
Changes
.clang-formatto fix some format error on Windows.For significant contributions please make sure you have completed the following items:
CHANGELOG.mdupdated for non-trivial changes