-
-
Notifications
You must be signed in to change notification settings - Fork 203
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
Sentry dotnet SDK is blowing away User.IpAddress if we set it explicitly and SendDefaultPii is true #3337
Comments
Hi @nicholashead , thanks for raising this. SystemWebRequestEventProcessorAt first glance, I think you're right about the I did notice we have this test which is supposed to avoid the situation you're describing (you set the
That event processor gets set on the SentryClient so I believe it runs for both ASP.NET Core and ASP.NET. I think it's getting overridden in ASP.NET by the code you highlighted in REMOTE_ADDR@nicholashead this won't affect you as you're setting the IP Address explicitly yourselves, but for ASP.NET Core Sentry also has these (which ensure Automatic IP address resolution works):
sentry-dotnet/src/Sentry.AspNetCore/ScopeExtensions.cs Lines 152 to 157 in 749c9a7
When building classic ASP.NET (framework) applications none of the mechanics to populate information in the scope seems to exist. So we probably need to wire up @bruno-garcia / @bitsandfoxes is there any additional context for whether/how this should work on ASP.NET? I can't see any equivalent in ASP.NET of the ASP.NET Core scope extensions, which populate this information. It's slightly more work to obtain this information in ASP.NET as we'd need to read it from headers ourselves but I think we could at the very least check for an |
Thanks for the research! Really appreciate it. The only reason we’re having issues with this is because of the proxied traffic, so if you were able to improve the default logic to look at the forwarded for header that’d be great. We’re still stuck on older .NET framework for one of our apps for now… |
There is something off with either the order of processor execution here or the processor is not supposed to simply overwrite data that has already been set.
Wdym? Doesn't the application of the scope be taken care of by the client? As part of the regular event processing in the core package? |
We have some ScopeExtensions for ASP.NET Core that get trigged by the SentryMiddleware. I can't see any equivalent of this for ASP.NET. |
Package
Sentry
.NET Flavor
.NET
.NET Version
4.7.2
OS
Windows
SDK Version
4.4.0
Self-Hosted Sentry Version
No response
Steps to Reproduce
Expected Result
The Sentry dashboard should show the User's IP as the one we set.
Actual Result
It appears to always contain an internal/server IP instead. I believe this may be because of this code?
sentry-dotnet/src/Sentry.AspNet/Internal/SystemWebRequestEventProcessor.cs
Line 75 in 5bcb9ca
But I am honestly not certain.
If I am doing something wrong here, would love guidance. But we have our own logic that fires off before a Sentry event sends - and if it's inside a web requests - gets the current user context (who's logged in) and IP address data, then stamps it onto the SentryEvent. We get the IP address ourselves because we're parsing CloudFlare/proxy IPs in the HTTP headers.
The text was updated successfully, but these errors were encountered: