search

getsentry / sentry-dotnet

Sentry SDK for .NET
https://docs.sentry.io/platforms/dotnet
MIT License
574 stars 205 forks source link

Sentry.Serilog doesn't log Environment #290

Closed DR9885 closed 4 years ago

DR9885 commented 4 years ago

Please mark the type framework used:

Please mark the type of the runtime used:

Please mark the NuGet packages used:

Please describe the the steps to reproduce the issue or link to a repository with a small reproducible code.

Currently we use Sentry.Serilog 1.2.0, and it does not correctly display Environment, it always displays "All Environments". We have "ASPNETCORE_ENVIRONMENT" set in the environment variables. We also use Serilog to distribute our logging, and have it configured with the config listed below.

Note: We dont use Sentry.AspNetCore for 2 reasons, 1 our console applications don't use AspNetCore it just uses NetCore, and 2 we use Serilog to enrich data meta data and to distribute our logging.

Also note, I changed the key below for security purposes.

  "Serilog": {
    "MinimumLevel": {
      "Default": "Information",
      "Override": {
        "Microsoft": "Warning",
        "System": "Warning"
      }
    },
    "WriteTo": [
      { "Name": "LiterateConsole" },
      {
        "Name": "Logentries",
        "Args": {
          "token": "99b37aaf-c472-40f7-a50f-5ab3d457f071-123",
          "outputTemplate": "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u3}] [{MachineName}] [{SourceContext}] {Message:lj}{NewLine}{Exception}"
        }
      },
      {
        "Name": "Sentry",
        "Args": {
          "MinimumBreadcrumbLevel": "Information",
          "MinimumEventLevel": "Error",
          "Dsn": "https://a16b8602ba8e46a49188c7de65af9072123@sentry.io/202852123",
          "AttachStacktrace": true,
          "SendDefaultPii": false
        }
      }
    ]
  }
bruno-garcia commented 4 years ago

Thanks for raising this. Sentry.Serilog is unaware of ASPNETCORE_ENVIRONMENT. That variable is supported by Sentry.AspNetCore because it relies on the framework's configuration system and that is what reads this env var and sets to IHostingEnvironment which the SDK reads from.

You can define the environment via SENTRY_ENVIRONMENT:

https://github.com/getsentry/sentry-dotnet/blob/f6007f621d4935ca2570c0163ede944d7c86483f/src/Sentry/SentryOptions.cs#L178

That'll be read by any Sentry.* NuGet package as it's part of the core of the SDK. You can also set it via the config file (less convenient):

        "Args": {
          "MinimumBreadcrumbLevel": "Information",
          "Environment": "Production"
        }

Also, note that you can mix Sentry.AspNetCore and Sentry.Serilog. We do that in this sample..

I hope this clarifies. Let us know otherwise.

AjaySewradj commented 4 years ago

I have the exact same problem. When the error gets logged, it's missing a tag with the Hosting Environment (Development, Production). We also only use the package Sentry.Serilog 1.2.0

I tried the following 2 fixes:

  1. [Does not work] I configured the Environment in the appsettings.json, per your suggestion (cleaned up a bit):

    "WriteTo": [
  {
    "Name": "Sentry",
    "Args": {
      "Dsn": "https://xxxxx",
      "Environment": "Production",
      "MinimumBreadcrumbLevel": "Information",
      "MinimumEventLevel": "Warning",
      "IncludeRequestPayload": true,
      "SendDefaultPii": true,
      "IncludeActivityData": true,
      "AttachStackTrace": true
    }
  }
]

  2. [WORKS] I set the environment variable in program.cs:

    var environment = Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT") ?? "Development";
Environment.SetEnvironmentVariable("SENTRY_ENVIRONMENT", environment);

I would prefer the approach from the appsettings.json. @bruno-garcia Do you have any idea why that doesn't work at the moment?

bruno-garcia commented 4 years ago

Do you have both Sentry.AspNetCore and Sentry.Serilog?

I have it here: https://ithub.com/getsentry/symbol-collector And here: https://github.com/NuGetTrends/nuget-trends

And in both cases I get the environment on all events.

Could you please provide a small repro? A repository with code I can run and debug though?

AjaySewradj commented 4 years ago

No, just Sentry.Serilog (v1.2.0). I also don't have a "Sentry" section in my appsettings.json since the docs say that specifying a dsn in the Serilog sink for Sentry (WriteTo..) would be enough to initialize Sentry. https://docs.sentry.io/platforms/dotnet/serilog/#configuration

I'll look into setting up a repro scenario, that will take some time. Thanks!

bruno-garcia commented 4 years ago

Why wouldn't you use Sentry.AspNetCore? It'll capture a new set of scenarios. Including request headers, unhandled exceptions in the middleware pipeline, request body (if opt-in), and more.

I also don't have a "Sentry" section in my appsettings.json since the docs say that specifying a dsn in the Serilog sink for Sentry (WriteTo..) would be enough to initialize Sentry.

That's true. You can just init via code if you prefer.

rmsy commented 4 years ago

Sorry to bump this old issue, but I am seeing this behavior as well when configuring the Sentry sink in appsettings.json:

      {
        "Name": "Sentry",
        "Args": {
          "Dsn": "<replaced>",
          "Environment": "QA",
          "MinimumBreadcrumbLevel": "Debug",
          "MinimumEventLevel": "Error",
          "AttachStackTrace": true,
          "DiagnosticsLevel": "Error"
        }
      }

No environment tag is being associated with the events.

P.S. This is a service, not a web app, so using the Sentry.AspNetCore NuGet is not relevant here. My other projects that are web apps, and use the AspNetCore NuGet, do not experience this issue.

AjaySewradj commented 4 years ago

@rmsy

I fixed it by implementing the following workaround in the main function of the webapp/service:

var environment = Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT") ?? "Production";
Environment.SetEnvironmentVariable("SENTRY_ENVIRONMENT", environment);
JimHume commented 4 years ago

@rmsy Any chance you could expand the list of parameters with your commit? I would like to use other options and I don't understand why loading through json is a second-class-citizen compared to hard-coding things in .cs files.

I too am running a service and prefer to use appsettings.json / the IConfiguration path for configuring Serilog. Sentry will not adhere to the majority of settings passed to it, however.

Other items that would be nice to have:

     {
        "Name": "Sentry",
        "Args": {
          "Dsn": "https://xxx@xxx/#",
          "Environment": "staging",
          "ServerName": "MyServerNameEvenIfItsAPod",
          "Release": "0.0.1",
          "MinimumBreadcrumbLevel": "Verbose",
          "MaxBreadcrumbs": 20,
          "MinimumEventLevel": "Information",
          "ReportAssemblies": false
        }
      }
JimHume commented 4 years ago

I noticed that PR 374 shows up here in the list of linked pull requests but mine doesn't. I also noticed that my PR didn't auto-link to this case. I can't seem to edit / update the PR to point here.

How does one link the issue to the PR and/or the PR to the issue? Does this have to be done when the PR is first created?

bruno-garcia commented 4 years ago

You can link by using the # like: Issue resolved by #380

Thanks @JimHume for all the work!

it's on NuGet as 2.1.2-beta

JimHume commented 4 years ago

Ah, perfect--thanks @bruno-garcia !