Skip to content

Update PortablePdbReader.cs#4

Merged
jamescrosswell merged 5 commits intogetsentry:mainfrom
jamescrosswell:fix-getmetadata-reader
Apr 24, 2024
Merged

Update PortablePdbReader.cs#4
jamescrosswell merged 5 commits intogetsentry:mainfrom
jamescrosswell:fix-getmetadata-reader

Conversation

@jamescrosswell
Copy link
Collaborator

Fixes an issue in PortablePdbReader.GetMetadataReader.

This method is supposed to return a value from the cache (if one exists) and otherwise try to create one from the assemblyPath.

The previous code only ever did any work or added anything to the cache if the following condition was satisfied:

 if (!_cache.TryGetValue(assemblyPath, out var provider) && provider is not null)

So if it couldn't find anything from the cache and also, if that thing it didn't find isn't null (which would never be possible - if it couldn't find anything, the out value will always be null).

jamescrosswell added a commit that referenced this pull request Dec 18, 2023
Applied fix from #4 in our local fork of the module.
jamescrosswell added a commit to getsentry/sentry-dotnet that referenced this pull request Dec 18, 2023
@bruno-garcia
Copy link
Member

CI is failing with:

error NU1903: Warning As Error: Package 'Microsoft.NETCore.App' 2.1.0 has a known high severity vulnerability,

Might need some fixing too (drop support for example?)

@bruno-garcia
Copy link
Member

Worth opening a PR upstream on https://github.com/benaadams/Ben.Demystifier too

bruno-garcia added a commit to getsentry/sentry-dotnet that referenced this pull request Jan 8, 2024
* Added basic metric types

* Moved Metric classes

* Added Increment aggregator

* Implemented Gauge metric

* Implemented Distribution and Set aggregations

* Added Metrics to ISentryClient API

* Update Hub.cs

* Verify tests

* Basic flush loop (no tests yet)

* Implemented statsd serialization

* Update CHANGELOG.md

* Split tests for Aggregagtor and BucketHelper

* Update MetricBucketHelperTests.cs

* Integrated review feedback

* Create MetricTests.verify.cs

* Updated verify tests

* Added Timing

* Added a (commented out) test to check if the aggregator is threadsafe

* Fixed concurrency issue (could still make this more performant)

* Reduced the scope of the lock for updating metrics

* Fixed unit tests

* Update MetricAggregator.cs

* Initial implementation of Code Locations

* Update Program.cs

* Updated solution filters

* Update CHANGELOG.md

* Changed Flush to FlushAsync

* Metrics now get flushed properly when disposing of the Hub

* Fixed serialization for code locations

* Update Timing.cs

* Clear stale seen periods at the end of each day

* Update CodeLocations.cs

* Fixed stacklevel when calling one of the two Timing constructors

* Removed IAsyncDisposable from MetricAggregator

* Cherry picked getsentry/Ben.Demystifier#4

* Update Ben.Demystifier

* Get line numbers with stack traces without enhanced stack traces

* Update Ben.Demystifier

* Reversed changes to AspNetCore.Basic sample (unrelated to this PR)

* Update Program.cs

* Improved the lock when incrementing/adding to existing metrics

* Tweaking docs

* Update CHANGELOG.md

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>

* Integrating review feedback

* Source generated RegEx in metric helper

* Update Envelope.cs

* Review feedback

* Integrating review feedback

* More performant string delimited tags used in the bucket key

* Integrating review feedback

* Integrated review feedback

* Removed unused private field

---------

Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>
stackTrace = LineEndingsHelper.RemoveLineEndings(stackTrace);
var trace = string.Join(string.Empty, stackTrace.Split(new[] { Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries));

#if (NETFRAMEWORK && !DEBUG)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears unrelated to the changes in this PR btw... I suspect something obscure changed in the .NET Framework such that this is now required.

@jamescrosswell jamescrosswell merged commit d7d639a into getsentry:main Apr 24, 2024
jamescrosswell added a commit that referenced this pull request Apr 24, 2024
Applied fix from #4 in our local fork of the module.

---------

Co-authored-by: Michal Strehovský <MichalStrehovsky@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants