Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add TransactionNameProvider to allow the name definition from Unknown transactions on ASP.NET Core #1421

Merged
merged 16 commits into from
Jan 28, 2022

Conversation

lucas-zimerman
Copy link
Collaborator

@lucas-zimerman lucas-zimerman commented Jan 4, 2022

  • The idea is to allow the user to set the transaction name on cases where the SDK isn't able to define the current Transaction name.

  • I'm also altering the GCP Integration since it already uses the integration, but without relying on the HttpContext to set the name.

Solves #1373.

@codecov-commenter
Copy link

codecov-commenter commented Jan 10, 2022

Codecov Report

Merging #1421 (5b04cf0) into main (782bdfd) will decrease coverage by 0.62%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1421      +/-   ##
==========================================
- Coverage   83.41%   82.79%   -0.63%     
==========================================
  Files         219      219              
  Lines        7370     7374       +4     
  Branches     1411     1412       +1     
==========================================
- Hits         6148     6105      -43     
- Misses        790      840      +50     
+ Partials      432      429       -3     
Impacted Files Coverage Δ
...try.AspNetCore/Extensions/HttpContextExtensions.cs 83.33% <100.00%> (ø)
src/Sentry.AspNetCore/SentryAspNetCoreOptions.cs 100.00% <100.00%> (ø)
src/Sentry.AspNetCore/SentryTracingMiddleware.cs 77.38% <100.00%> (+0.55%) ⬆️
src/Sentry.Google.Cloud.Functions/SentryStartup.cs 97.95% <100.00%> (+0.04%) ⬆️
src/Sentry/PlatformAbstractions/FrameworkInfo.cs 0.00% <0.00%> (-100.00%) ⬇️
...ntry/PlatformAbstractions/RegistryKeyExtensions.cs 0.00% <0.00%> (-50.00%) ⬇️
...ntry/Integrations/NetFxInstallationsIntegration.cs 28.57% <0.00%> (-28.58%) ⬇️
...rmAbstractions/NetFxInstallationsEventProcessor.cs 4.54% <0.00%> (-18.19%) ⬇️
...Sentry/PlatformAbstractions/FrameworkInfo.NetFx.cs 0.00% <0.00%> (-15.50%) ⬇️
...ntry/PlatformAbstractions/FrameworkInstallation.cs 25.00% <0.00%> (-12.50%) ⬇️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 782bdfd...5b04cf0. Read the comment docs.

@lucas-zimerman lucas-zimerman changed the title PoC: Expose Sentry route and add GCP as sample. Add ITransactionNameProvider to allow the name definition from Unknown transactions on ASP.NET Core Jan 10, 2022
@lucas-zimerman lucas-zimerman marked this pull request as ready for review January 12, 2022 17:18
Copy link
Contributor

@SimonCropp SimonCropp left a comment

Choose a reason for hiding this comment

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

LGTM. i pushed some very small cosmetic changes

CHANGELOG.md Outdated Show resolved Hide resolved
@lucas-zimerman lucas-zimerman changed the title Add ITransactionNameProvider to allow the name definition from Unknown transactions on ASP.NET Core Add TransactionNameProvider to allow the name definition from Unknown transactions on ASP.NET Core Jan 21, 2022
@SimonCropp
Copy link
Contributor

do we need a corresponding docs PR?

@lucas-zimerman
Copy link
Collaborator Author

do we need a corresponding docs PR?

We deffo need some docs but I believe we could work on it once we have a released version with this PR bundled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants