Jaeger Exporter: Fixed CurrentBatches dictionary being modified while a flush is running. #571
Conversation
This file contains 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
CodeBlanch
requested review from
bruno-garcia,
discostu105,
lmolkova,
MikeGoldsmith and
SergeyKanzhelev
as code owners
SergeyKanzhelev
approved these changes
Apr 6, 2020
cijothomas
approved these changes
Apr 6, 2020
MikeGoldsmith
pushed a commit
to MikeGoldsmith/opentelemetry-dotnet
that referenced
this pull request
Apr 9, 2020
…ng. (open-telemetry#571) Co-authored-by: Sergey Kanzhelev <S.Kanzhelev@live.com>
MikeGoldsmith
added a commit
to MikeGoldsmith/opentelemetry-dotnet
that referenced
this pull request
Apr 9, 2020
…et into context-propagation * 'master' of github.com:open-telemetry/opentelemetry-dotnet: Clone Activity tags to Span Attributes (open-telemetry#572) add LS access token header to http request (open-telemetry#587) ASP.NET dependency collector fixes and cleanup. (open-telemetry#570) Fixed up Jaeger project so that correct Thrift.dll is included in the nupkg & ApacheThrift is listed as a dependency. (open-telemetry#566) Fixed CurrentBatches dictionary being modified while a flush is running. (open-telemetry#571) Implemented short-live bound instruments. (open-telemetry#547) updating README.md samples (open-telemetry#556) Attempting to fix the myget build pipeline so the packages come out valid on the other end. (open-telemetry#565) Adding method to EvictingQueue, updating SpanSdk to update if key exists (open-telemetry#557) Fixed the solution not building from dotnet build CLI. (open-telemetry#554) fixes open-telemetry#559 (open-telemetry#563) Wrapping up enumerables for open-telemetry#407 and some cleanup (open-telemetry#558) Added support for .NET Framework HttpClient & HttpWebRequest dependency collection. (open-telemetry#553) remove UpdateThreadArguments class from LastValueAggregatorTest (open-telemetry#552) Fix build (open-telemetry#548) AspNet request collector (open-telemetry#543) # Conflicts: # OpenTelemetry.proj # samples/Exporters/AspNet/Properties/AssemblyInfo.cs # samples/Exporters/AspNet/Web.Debug.config # samples/Exporters/AspNet/Web.Release.config # src/OpenTelemetry.Api/Context/CorrelationContextBuilder.cs # src/OpenTelemetry.Api/Context/CorrelationContextEntry.cs # src/OpenTelemetry.Api/Context/DistributedContext.cs # src/OpenTelemetry.Api/Metrics/CounterMetric.cs # src/OpenTelemetry.Api/Metrics/NoOpCounterMetric.cs # src/OpenTelemetry/Metrics/CounterMetricSdk.cs # src/OpenTelemetry/Metrics/MeterSdk.cs # src/OpenTelemetry/Metrics/RecordStatus.cs # src/OpenTelemetry/Trace/SpanSdk.cs # test/OpenTelemetry.Collector.AspNet.Tests/xunit.runner.json # test/OpenTelemetry.Tests/Impl/Context/DistributedContextsScopeTest.cs # test/OpenTelemetry.Tests/Impl/Context/Propagation/DistributedContextDeserializationTest.cs # test/OpenTelemetry.Tests/Impl/Context/Propagation/DistributedContextSerializationTest.cs # test/OpenTelemetry.Tests/Impl/DistributedContext/DistributedContextBuilderTest.cs # test/OpenTelemetry.Tests/Impl/DistributedContext/DistributedContextTest.cs # test/OpenTelemetry.Tests/Impl/DistributedContext/Propagation/DistributedContextRoundtripTest.cs # test/OpenTelemetry.Tests/Impl/Metrics/CounterCleanUpTests.cs # test/OpenTelemetry.Tests/Impl/Metrics/MetricsTest.cs
SergeyKanzhelev
added a commit
that referenced
this pull request
Apr 14, 2020
* consolidate all files under DistributeContext to Context in API and SDK * rename DistributedContext classes to CorrelationContext et al * update missed test renaming * re-add DistributedContext and refactor carrier to use it * add IEquatable to DistributedContext * AspNet request collector (#543) * Added a collector for incoming ASP.NET requests. * Unit tests. * Code review. * Attempting to get windows build working. * Attempting to fix build. * Attempting to get the build working. * Attempting to get the build working. * Attempting to fix the build. * Attempting to get Linux build working. * Attempting to get linux build working. * Attempting to get linux build working. * Attempting to get build working. * Attempting to get all tests running on windows. * Attempting to get tests running on windows. * Attempting to get the myget build working. * Indention fix in myget yml. * Code review. * Code review. * Code review. * Code review. * Code review. * Fix build (#548) * fix build * rename queue to pool * do not pack tests * try to switch to VSBuild * rename CorrelationContextScopeTest to DistributedContextScopeTest * remove UpdateThreadArguments class from LastValueAggregatorTest (#552) * Added support for .NET Framework HttpClient & HttpWebRequest dependency collection. (#553) * Added support for .NET Framework HttpClient & HttpWebRequest dependency collection. * Code review feedback. * Updated HttpHandlerDiagnosticListener with latest changes from dotnet/runtime. Added try/finally for span.End calls. Fixed missing span.End in Sql exception event. * Fixed broken SqlClientTests. * A few tweaks to README for recent collector changes. * Tested Azure SDK exception path and updated the TODO comment. * Wrapping up enumerables for #407 and some cleanup (#558) * Wrapping up enumerables for #407 and some cleanup * Fixing whitespaces * One more Co-authored-by: Sergey Kanzhelev <S.Kanzhelev@live.com> * fixes #559 (#563) * Fixed the solution not building from dotnet build CLI. (#554) Co-authored-by: Sergey Kanzhelev <S.Kanzhelev@live.com> * Adding method to EvictingQueue, updating SpanSdk to update if key exists (#557) * Adding method to EvictingQueue, updating SpanSdk to update if key exists * Adding Replacing test to EvictingQueue; Updating SpanTest; Renaming from Update to Replace * Adding wrapper to AddOrUpdateAttribute, adding validation to index in Replace method * Adding NoReplacing Test when element doesn't exist Co-authored-by: Sergey Kanzhelev <S.Kanzhelev@live.com> * Attempting to fix the myget build pipeline so the packages come out valid on the other end. (#565) I don't have a way to start a build from fork. So let's test right in master... * updating README.md samples (#556) * updating README.md samples * adding missing variables to example * updating samples in readme Co-authored-by: Mike Goldsmith <goldsmith.mike@gmail.com> * Implemented short-live bound instruments. (#547) * Initial draft for implementing record cleanup. Implemented for longCounters only. One basic test as well. * small fixes * Test for double counter * Double test counter * Use instrument level lock * Added comment. * Tests fix * minor * Added new intermediate status NoPendingUpdate for instrument record. This greatly reduced the need to take locks * comment * Fixed CurrentBatches dictionary being modified while a flush is running. (#571) Co-authored-by: Sergey Kanzhelev <S.Kanzhelev@live.com> * Fixed up Jaeger project so that correct Thrift.dll is included in the nupkg & ApacheThrift is listed as a dependency. (#566) Co-authored-by: Sergey Kanzhelev <S.Kanzhelev@live.com> * ASP.NET dependency collector fixes and cleanup. (#570) * Updated with latest bug fixes. Refactored to fit into OT solution. * Ported bug fixes and unit tests. * Code review. * Unit test improvement. * Code review. * Fixed crashing TestServer on close. Co-authored-by: Sergey Kanzhelev <S.Kanzhelev@live.com> * add LS access token header to http request (#587) * Clone Activity tags to Span Attributes (#572) Resolves #430 * Initial work for validation * Changed activity tags creation Co-authored-by: Mike Goldsmith <goldsmith.mike@gmail.com> * consolidate all files under DistributeContext to Context in API and SDK * rename DistributedContext classes to CorrelationContext et al * update missed test renaming * re-add DistributedContext and refactor carrier to use it * Fix build (#548) * fix build * rename queue to pool * do not pack tests * try to switch to VSBuild * replace CorrelationContext with DistributedContext in BinarySerializer * revert whitespace fixes to simplify review * Update src/OpenTelemetry.Api/Context/CorrelationContext.cs Co-Authored-By: Bruno Garcia <bruno@brunogarcia.com> * Update src/OpenTelemetry.Api/Context/DistributedContextBuilder.cs Co-Authored-By: Bruno Garcia <bruno@brunogarcia.com> Co-authored-by: Mikel Blanchard <mblanchard@macrosssoftware.com> Co-authored-by: Sergey Kanzhelev <S.Kanzhelev@live.com> Co-authored-by: Alexey Zimarev <alex@zimarev.com> Co-authored-by: Eddy Nakamura <eddynaka@gmail.com> Co-authored-by: Eddy Nakamura <ednakamu@microsoft.com> Co-authored-by: Cijo Thomas <cithomas@microsoft.com> Co-authored-by: Bruno Santos <brunomiguelas@gmail.com> Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>
Yun-Ting
pushed a commit
to Yun-Ting/opentelemetry-dotnet
that referenced
this pull request
Oct 13, 2022
Co-authored-by: Cijo Thomas <cithomas@microsoft.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The Jaeger flush logic is enumerating over the batches to be sent so if a new one comes in while we're doing that we get an enumeration modified exception. Moved the logic into the lock in AppendAsync to prevent that.