Reorder the destructor of members in LoggerProvider and TracerProvider#1245
Merged
ThomsonTan merged 8 commits intoopen-telemetry:mainfrom Mar 8, 2022
Merged
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1245 +/- ##
==========================================
+ Coverage 92.29% 92.31% +0.03%
==========================================
Files 198 198
Lines 7273 7281 +8
==========================================
+ Hits 6712 6721 +9
+ Misses 561 560 -1
|
esigo
reviewed
Mar 4, 2022
esigo
reviewed
Mar 4, 2022
Member
Author
|
It's too late on my timezone. I will solve the failed jobs tomorrow. |
esigo
reviewed
Mar 5, 2022
esigo
reviewed
Mar 5, 2022
lalitb
approved these changes
Mar 6, 2022
owent
force-pushed
the
fix_crash_when_destroying_tracer
branch
from
d9792f5 to
3b54c24
Compare
Member
|
@owent could you please rebase? |
ThomsonTan
reviewed
Mar 7, 2022
ThomsonTan
reviewed
Mar 7, 2022
| // order of declaration is important here - instrumentation library should destroy after | ||
| // logger-context. | ||
| std::unique_ptr<instrumentationlibrary::InstrumentationLibrary> instrumentation_library_; | ||
| std::shared_ptr<LoggerContext> context_; |
Contributor
There was a problem hiding this comment.
Is cyclic dependency still an issue here as it changes weak_ptr to shared_ptr?
Member
Author
There was a problem hiding this comment.
There is no cyclic dependency right now.It's only be hold in Logger and LoggerProvider and Logger is only hold by LoggerProvider.
ThomsonTan
reviewed
Mar 7, 2022
ThomsonTan
reviewed
Mar 7, 2022
| } | ||
|
|
||
| // Should only shutdown exporter ONCE. | ||
| if (!already_shutdown && exporter_ != nullptr) |
Contributor
There was a problem hiding this comment.
Is showing down is going, later call to Shutdown will just return true even if the previous Shutdown has not been completed?
Member
Author
There was a problem hiding this comment.
Yes and thanks, it's definitely a problem.I add a mutex lock to prevent this.
…r. It's safe to shutdown multiple times.(Fix open-telemetry#1244) Signed-off-by: owentou <owentou@tencent.com>
…comparison of integer expressions of different signedness: 'int' and 'const long unsigned int' [-Wsign-compare]` Signed-off-by: owentou <owentou@tencent.com>
…nd `SimpleSpanProcessor` Signed-off-by: owentou <owentou@tencent.com>
Signed-off-by: owentou <owentou@tencent.com>
Signed-off-by: owentou <owentou@tencent.com>
+ Temporary fix the `Export()` may be called too many times when we shutdown or just wakeup worker thread once.This problem is completely fixed in open-telemetry#1209 but not merged yet. + Fix http client may has a different error code when our network is under a proxy.
Signed-off-by: owentou <owentou@tencent.com>
owent
force-pushed
the
fix_crash_when_destroying_tracer
branch
from
3b54c24 to
67963f1
Compare
Member
Author
Done |
ThomsonTan
approved these changes
Mar 8, 2022
ThomsonTan
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…r. It's safe to shutdown multiple times.(Fix #1244)
Signed-off-by: owentou owentou@tencent.com
Fixes #1244
Changes
For significant contributions please make sure you have completed the following items:
CHANGELOG.mdupdated for non-trivial changes